diff options
Diffstat (limited to 'controller-server/src')
26 files changed, 703 insertions, 201 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index d68c5caf685..1d685895914 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -41,6 +41,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerato import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Deployment; +import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobList; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.JobStatus.JobRun; @@ -50,11 +51,11 @@ import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; import com.yahoo.vespa.hosted.controller.concurrent.Once; import com.yahoo.vespa.hosted.controller.deployment.DeploymentSteps; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; -import com.yahoo.vespa.hosted.controller.security.AccessControl; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.rotation.Rotation; import com.yahoo.vespa.hosted.controller.rotation.RotationLock; import com.yahoo.vespa.hosted.controller.rotation.RotationRepository; +import com.yahoo.vespa.hosted.controller.security.AccessControl; import com.yahoo.vespa.hosted.controller.security.Credentials; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; @@ -72,6 +73,7 @@ import java.time.Instant; import java.util.Collections; import java.util.Comparator; import java.util.EnumSet; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -336,7 +338,8 @@ public class ApplicationController { ActivateResult result = deploy(applicationId, applicationPackage, zone, options, rotationNames, cnames); lockOrThrow(applicationId, application -> - store(application.withNewDeployment(zone, applicationVersion, platformVersion, clock.instant()))); + store(application.withNewDeployment(zone, applicationVersion, platformVersion, clock.instant(), + warningsFrom(result)))); return result; } } @@ -761,4 +764,17 @@ public class ApplicationController { .max(naturalOrder()); } + /** Extract deployment warnings metric from deployment result */ + private static Map<DeploymentMetrics.Warning, Integer> warningsFrom(ActivateResult result) { + if (result.prepareResponse().log == null) return Map.of(); + Map<DeploymentMetrics.Warning, Integer> warnings = new HashMap<>(); + for (Log log : result.prepareResponse().log) { + // TODO: Categorize warnings. Response from config server should be updated to include the appropriate + // category and typed log level + if (!"warn".equalsIgnoreCase(log.level) && !"warning".equalsIgnoreCase(log.level)) continue; + warnings.merge(DeploymentMetrics.Warning.all, 1, Integer::sum); + } + return Collections.unmodifiableMap(warnings); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index 61c10abce14..0cfbc60ad8c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -148,14 +148,14 @@ public class LockedApplication { } public LockedApplication withNewDeployment(ZoneId zone, ApplicationVersion applicationVersion, Version version, - Instant instant) { + Instant instant, Map<DeploymentMetrics.Warning, Integer> warnings) { // Use info from previous deployment if available, otherwise create a new one. Deployment previousDeployment = deployments.getOrDefault(zone, new Deployment(zone, applicationVersion, version, instant)); Deployment newDeployment = new Deployment(zone, applicationVersion, version, instant, previousDeployment.clusterUtils(), previousDeployment.clusterInfo(), - previousDeployment.metrics(), + previousDeployment.metrics().with(warnings), previousDeployment.activity()); return with(newDeployment); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentActivity.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentActivity.java index 881de040e28..03c08509a5e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentActivity.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentActivity.java @@ -63,7 +63,7 @@ public class DeploymentActivity { public static DeploymentActivity create(Optional<Instant> queriedAt, Optional<Instant> writtenAt, OptionalDouble lastQueriesPerSecond, OptionalDouble lastWritesPerSecond) { - if (!queriedAt.isPresent() && !writtenAt.isPresent()) { + if (queriedAt.isEmpty() && writtenAt.isEmpty()) { return none; } return new DeploymentActivity(queriedAt, writtenAt, lastQueriesPerSecond, lastWritesPerSecond); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentMetrics.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentMetrics.java index 1ee461cbb8d..7a50184e7a4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentMetrics.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentMetrics.java @@ -2,16 +2,20 @@ package com.yahoo.vespa.hosted.controller.application; import java.time.Instant; +import java.util.Map; +import java.util.Objects; import java.util.Optional; /** - * Metrics for a deployment of an application. + * Metrics for a deployment of an application. This contains a snapshot of metrics gathered at a point in time, it does + * not contain any historical data. * * @author smorgrav + * @author mpolden */ public class DeploymentMetrics { - public static final DeploymentMetrics none = new DeploymentMetrics(0, 0, 0, 0, 0); + public static final DeploymentMetrics none = new DeploymentMetrics(0, 0, 0, 0, 0, Optional.empty(), Map.of()); private final double queriesPerSecond; private final double writesPerSecond; @@ -19,44 +23,97 @@ public class DeploymentMetrics { private final double queryLatencyMillis; private final double writeLatencyMills; private final Optional<Instant> instant; + private final Map<Warning, Integer> warnings; + /* DO NOT USE. Public for serialization purposes */ public DeploymentMetrics(double queriesPerSecond, double writesPerSecond, double documentCount, - double queryLatencyMillis, double writeLatencyMills) { - this(queriesPerSecond, writesPerSecond, documentCount, queryLatencyMillis, writeLatencyMills, Optional.empty()); - } - - public DeploymentMetrics(double queriesPerSecond, double writesPerSecond, double documentCount, - double queryLatencyMillis, double writeLatencyMills, Optional<Instant> instant) { + double queryLatencyMillis, double writeLatencyMills, Optional<Instant> instant, + Map<Warning, Integer> warnings) { this.queriesPerSecond = queriesPerSecond; this.writesPerSecond = writesPerSecond; this.documentCount = documentCount; this.queryLatencyMillis = queryLatencyMillis; this.writeLatencyMills = writeLatencyMills; - this.instant = instant; + this.instant = Objects.requireNonNull(instant, "instant must be non-null"); + this.warnings = Map.copyOf(Objects.requireNonNull(warnings, "warnings must be non-null")); + if (warnings.entrySet().stream().anyMatch(kv -> kv.getValue() < 0)) { + throw new IllegalArgumentException("Warning count must be non-negative. Got " + warnings); + } } + /** Returns the number of queries per second */ public double queriesPerSecond() { return queriesPerSecond; } + /** Returns the number of writes per second */ public double writesPerSecond() { return writesPerSecond; } + /** Returns the number of documents */ public double documentCount() { return documentCount; } + /** Returns the average query latency in milliseconds */ public double queryLatencyMillis() { return queryLatencyMillis; } + /** Returns the average write latency in milliseconds */ public double writeLatencyMillis() { return writeLatencyMills; } + /** Returns the approximate time this was measured */ public Optional<Instant> instant() { return instant; } + /** Returns the number of warnings of the most recent deployment */ + public Map<Warning, Integer> warnings() { + return warnings; + } + + public DeploymentMetrics withQueriesPerSecond(double queriesPerSecond) { + return new DeploymentMetrics(queriesPerSecond, writesPerSecond, documentCount, queryLatencyMillis, + writeLatencyMills, instant, warnings); + } + + public DeploymentMetrics withWritesPerSecond(double writesPerSecond) { + return new DeploymentMetrics(queriesPerSecond, writesPerSecond, documentCount, queryLatencyMillis, + writeLatencyMills, instant, warnings); + } + + public DeploymentMetrics withDocumentCount(double documentCount) { + return new DeploymentMetrics(queriesPerSecond, writesPerSecond, documentCount, queryLatencyMillis, + writeLatencyMills, instant, warnings); + } + + public DeploymentMetrics withQueryLatencyMillis(double queryLatencyMillis) { + return new DeploymentMetrics(queriesPerSecond, writesPerSecond, documentCount, queryLatencyMillis, + writeLatencyMills, instant, warnings); + } + + public DeploymentMetrics withWriteLatencyMillis(double writeLatencyMills) { + return new DeploymentMetrics(queriesPerSecond, writesPerSecond, documentCount, queryLatencyMillis, + writeLatencyMills, instant, warnings); + } + + public DeploymentMetrics at(Instant instant) { + return new DeploymentMetrics(queriesPerSecond, writesPerSecond, documentCount, queryLatencyMillis, + writeLatencyMills, Optional.of(instant), warnings); + } + + public DeploymentMetrics with(Map<Warning, Integer> warnings) { + return new DeploymentMetrics(queriesPerSecond, writesPerSecond, documentCount, queryLatencyMillis, + writeLatencyMills, instant, warnings); + } + + /** Types of deployment warnings. We currently have only one */ + public enum Warning { + all + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index c64bc99d0c7..5c9489d415f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -604,7 +604,7 @@ public class InternalStepRunner implements StepRunner { " </container>\n" + "</services>\n"; - return servicesXml.getBytes(); + return servicesXml.getBytes(StandardCharsets.UTF_8); } /** Returns a dummy deployment xml which sets up the service identity for the tester, if present. */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java index 506231f086b..87c0ed8b9ab 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java @@ -86,7 +86,7 @@ public class ControllerMaintenance extends AbstractComponent { contactInformationMaintainer = new ContactInformationMaintainer(controller, Duration.ofHours(12), jobControl, contactRetriever); costReportMaintainer = new CostReportMaintainer(controller, Duration.ofHours(2), reportConsumer, jobControl, nodeRepositoryClient, Clock.systemUTC(), selfHostedCostConfig); routingPolicyMaintainer = new RoutingPolicyMaintainer(controller, Duration.ofMinutes(5), jobControl, nameService, curator); - resourceMeterMaintainer = new ResourceMeterMaintainer(controller, Duration.ofMinutes(5), jobControl, nodeRepositoryClient, Clock.systemUTC(), metric, resourceSnapshotConsumer); + resourceMeterMaintainer = new ResourceMeterMaintainer(controller, Duration.ofMinutes(60), jobControl, nodeRepositoryClient, Clock.systemUTC(), metric, resourceSnapshotConsumer); } public Upgrader upgrader() { return upgrader; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java index c3474a485ca..787a050e59e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java @@ -11,10 +11,10 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.RotationStatus; import java.time.Duration; +import java.time.Instant; import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.TreeMap; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; @@ -61,19 +61,20 @@ public class DeploymentMetricsMaintainer extends Maintainer { applications.store(locked.withRotationStatus(rotationStatus(application)))); for (Deployment deployment : application.deployments().values()) { - MetricsService.DeploymentMetrics deploymentMetrics = controller().metricsService() - .getDeploymentMetrics(application.id(), deployment.zone()); - - DeploymentMetrics newMetrics = new DeploymentMetrics(deploymentMetrics.queriesPerSecond(), - deploymentMetrics.writesPerSecond(), - deploymentMetrics.documentCount(), - deploymentMetrics.queryLatencyMillis(), - deploymentMetrics.writeLatencyMillis(), - Optional.of(controller().clock().instant())); - - applications.lockIfPresent(application.id(), locked -> - applications.store(locked.with(deployment.zone(), newMetrics) - .recordActivityAt(controller().clock().instant(), deployment.zone()))); + MetricsService.DeploymentMetrics collectedMetrics = controller().metricsService() + .getDeploymentMetrics(application.id(), deployment.zone()); + Instant now = controller().clock().instant(); + applications.lockIfPresent(application.id(), locked -> { + DeploymentMetrics newMetrics = locked.get().deployments().get(deployment.zone()).metrics() + .withQueriesPerSecond(collectedMetrics.queriesPerSecond()) + .withWritesPerSecond(collectedMetrics.writesPerSecond()) + .withDocumentCount(collectedMetrics.documentCount()) + .withQueryLatencyMillis(collectedMetrics.queryLatencyMillis()) + .withWriteLatencyMillis(collectedMetrics.writeLatencyMillis()) + .at(now); + applications.store(locked.with(deployment.zone(), newMetrics) + .recordActivityAt(now, deployment.zone())); + }); } } catch (Exception e) { failures.incrementAndGet(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java index 07680fc8b72..037e7d8ced6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java @@ -13,6 +13,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.chef.rest.PartialNode; import com.yahoo.vespa.hosted.controller.api.integration.chef.rest.PartialNodeResult; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.ApplicationList; +import com.yahoo.vespa.hosted.controller.application.Deployment; +import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobList; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.rotation.RotationLock; @@ -20,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.rotation.RotationLock; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -33,12 +36,13 @@ import java.util.stream.Collectors; */ public class MetricsReporter extends Maintainer { - public static final String convergeMetric = "seconds.since.last.chef.convergence"; - public static final String deploymentFailMetric = "deployment.failurePercentage"; - public static final String deploymentAverageDuration = "deployment.averageDuration"; - public static final String deploymentFailingUpgrades = "deployment.failingUpgrades"; - public static final String deploymentBuildAgeSeconds = "deployment.buildAgeSeconds"; - public static final String remainingRotations = "remaining_rotations"; + public static final String CONVERGENCE_METRIC = "seconds.since.last.chef.convergence"; + public static final String DEPLOYMENT_FAIL_METRIC = "deployment.failurePercentage"; + public static final String DEPLOYMENT_AVERAGE_DURATION = "deployment.averageDuration"; + public static final String DEPLOYMENT_FAILING_UPGRADES = "deployment.failingUpgrades"; + public static final String DEPLOYMENT_BUILD_AGE_SECONDS = "deployment.buildAgeSeconds"; + public static final String DEPLOYMENT_WARNINGS = "deployment.warnings"; + public static final String REMAINING_ROTATIONS = "remaining_rotations"; private final Metric metric; private final Chef chefClient; @@ -69,7 +73,7 @@ public class MetricsReporter extends Maintainer { private void reportRemainingRotations() { try (RotationLock lock = controller().applications().rotationRepository().lock()) { int availableRotations = controller().applications().rotationRepository().availableRotations(lock).size(); - metric.set(remainingRotations, availableRotations, metric.createContext(Collections.emptyMap())); + metric.set(REMAINING_ROTATIONS, availableRotations, metric.createContext(Collections.emptyMap())); } } @@ -104,7 +108,7 @@ public class MetricsReporter extends Maintainer { Optional<String> environment = node.getValue("environment"); Optional<String> region = node.getValue("region"); - if(environment.isPresent() && region.isPresent()) { + if (environment.isPresent() && region.isPresent()) { dimensions.put("zone", String.format("%s.%s", environment.get(), region.get())); } @@ -112,7 +116,7 @@ public class MetricsReporter extends Maintainer { Optional<String> application = node.getValue("application"); application.ifPresent(app -> dimensions.put("app", String.format("%s.%s", app, node.getValue("instance").orElse("default")))); Metric.Context context = metric.createContext(dimensions); - metric.set(convergeMetric, secondsSinceConverge, context); + metric.set(CONVERGENCE_METRIC, secondsSinceConverge, context); } } @@ -120,21 +124,25 @@ public class MetricsReporter extends Maintainer { ApplicationList applications = ApplicationList.from(controller().applications().asList()) .hasProductionDeployment(); - metric.set(deploymentFailMetric, deploymentFailRatio(applications) * 100, metric.createContext(Collections.emptyMap())); + metric.set(DEPLOYMENT_FAIL_METRIC, deploymentFailRatio(applications) * 100, metric.createContext(Collections.emptyMap())); averageDeploymentDurations(applications, clock.instant()).forEach((application, duration) -> { - metric.set(deploymentAverageDuration, duration.getSeconds(), metric.createContext(dimensions(application))); + metric.set(DEPLOYMENT_AVERAGE_DURATION, duration.getSeconds(), metric.createContext(dimensions(application))); }); deploymentsFailingUpgrade(applications).forEach((application, failingJobs) -> { - metric.set(deploymentFailingUpgrades, failingJobs, metric.createContext(dimensions(application))); + metric.set(DEPLOYMENT_FAILING_UPGRADES, failingJobs, metric.createContext(dimensions(application))); + }); + + deploymentWarnings(applications).forEach((application, warnings) -> { + metric.set(DEPLOYMENT_WARNINGS, warnings, metric.createContext(dimensions(application))); }); for (Application application : applications.asList()) application.deploymentJobs().statusOf(JobType.component) .flatMap(JobStatus::lastSuccess) .flatMap(run -> run.application().buildTime()) - .ifPresent(buildTime -> metric.set(deploymentBuildAgeSeconds, + .ifPresent(buildTime -> metric.set(DEPLOYMENT_BUILD_AGE_SECONDS, controller().clock().instant().getEpochSecond() - buildTime.getEpochSecond(), metric.createContext(dimensions(application.id())))); } @@ -180,6 +188,21 @@ public class MetricsReporter extends Maintainer { .map(totalDuration -> totalDuration.dividedBy(jobDurations.size())) .orElse(Duration.ZERO); } + + private static Map<ApplicationId, Integer> deploymentWarnings(ApplicationList applications) { + return applications.asList().stream() + .collect(Collectors.toMap(Application::id, a -> maxWarningCountOf(a.deployments().values()))); + } + + private static int maxWarningCountOf(Collection<Deployment> deployments) { + return deployments.stream() + .map(Deployment::metrics) + .map(DeploymentMetrics::warnings) + .map(Map::values) + .flatMap(Collection::stream) + .max(Integer::compareTo) + .orElse(0); + } private static void keepNodesWithSystem(PartialNodeResult nodeResult, SystemName system) { nodeResult.rows.removeIf(node -> !system.name().equals(node.getValue("system").orElse("main"))); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 0c79c5893c6..8433b2f368c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -10,6 +10,7 @@ import com.yahoo.config.provision.HostName; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; +import com.yahoo.slime.ObjectTraverser; import com.yahoo.slime.Slime; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; @@ -140,6 +141,7 @@ public class ApplicationSerializer { private final String deploymentMetricsQueryLatencyField = "queryLatencyMillis"; private final String deploymentMetricsWriteLatencyField = "writeLatencyMillis"; private final String deploymentMetricsUpdateTime = "lastUpdated"; + private final String deploymentMetricsWarningsField = "warnings"; // ------------------ Serialization @@ -192,6 +194,10 @@ public class ApplicationSerializer { root.setDouble(deploymentMetricsQueryLatencyField, metrics.queryLatencyMillis()); root.setDouble(deploymentMetricsWriteLatencyField, metrics.writeLatencyMillis()); metrics.instant().ifPresent(instant -> root.setLong(deploymentMetricsUpdateTime, instant.toEpochMilli())); + if (!metrics.warnings().isEmpty()) { + Cursor warningsObject = root.setObject(deploymentMetricsWarningsField); + metrics.warnings().forEach((warning, count) -> warningsObject.setLong(warning.name(), count)); + } } private void clusterInfoToSlime(Map<ClusterSpec.Id, ClusterInfo> clusters, Cursor object) { @@ -360,7 +366,15 @@ public class ApplicationSerializer { object.field(deploymentMetricsDocsField).asDouble(), object.field(deploymentMetricsQueryLatencyField).asDouble(), object.field(deploymentMetricsWriteLatencyField).asDouble(), - instant); + instant, + deploymentWarningsFrom(object.field(deploymentMetricsWarningsField))); + } + + private Map<DeploymentMetrics.Warning, Integer> deploymentWarningsFrom(Inspector object) { + Map<DeploymentMetrics.Warning, Integer> warnings = new HashMap<>(); + object.traverse((ObjectTraverser) (name, value) -> warnings.put(DeploymentMetrics.Warning.valueOf(name), + (int) value.asLong())); + return Collections.unmodifiableMap(warnings); } private Map<HostName, RotationStatus> rotationStatusFromSlime(Inspector object) { @@ -376,9 +390,9 @@ public class ApplicationSerializer { return Collections.unmodifiableMap(rotationStatus); } - private Map<ClusterSpec.Id, ClusterInfo> clusterInfoMapFromSlime(Inspector object) { + private Map<ClusterSpec.Id, ClusterInfo> clusterInfoMapFromSlime (Inspector object) { Map<ClusterSpec.Id, ClusterInfo> map = new HashMap<>(); - object.traverse((String name, Inspector obect) -> map.put(new ClusterSpec.Id(name), clusterInfoFromSlime(obect))); + object.traverse((String name, Inspector value) -> map.put(new ClusterSpec.Id(name), clusterInfoFromSlime(value))); return map; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolver.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolver.java index 8a5e70af6c0..21c9875bb8b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolver.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolver.java @@ -93,7 +93,7 @@ public class AthenzRoleResolver implements RoleMembership.Resolver { memberships.add(Role.hostedOperator); } if (tenant.isPresent() && isTenantAdmin(identity, tenant.get())) { - memberships.add(Role.tenantAdmin).limitedTo(tenant.get().name()); + memberships.add(Role.athenzTenantAdmin).limitedTo(tenant.get().name()); } AthenzDomain principalDomain = identity.getDomain(); if (principalDomain.equals(SCREWDRIVER_DOMAIN)) { @@ -102,11 +102,11 @@ public class AthenzRoleResolver implements RoleMembership.Resolver { if (tenant.get() instanceof AthenzTenant) { AthenzDomain tenantDomain = ((AthenzTenant) tenant.get()).domain(); if (hasDeployerAccess(identity, tenantDomain, application.get())) { - memberships.add(Role.tenantPipelineOperator).limitedTo(tenant.get().name(), application.get()); + memberships.add(Role.tenantPipeline).limitedTo(tenant.get().name(), application.get()); } } else { - memberships.add(Role.tenantPipelineOperator).limitedTo(tenant.get().name(), application.get()); + memberships.add(Role.tenantPipeline).limitedTo(tenant.get().name(), application.get()); } } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java index 3776b1d2a13..e3d81e04591 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java @@ -17,10 +17,8 @@ import java.util.Set; */ public enum PathGroup { - /** Paths used for system management by operators */ + /** Paths used for system management by operators. */ operator("/controller/v1/{*}", - "/cost/v1/{*}", - "/deployment/v1/{*}", "/flags/v1/{*}", "/nodes/v2/{*}", "/orchestrator/v1/{*}", @@ -28,57 +26,84 @@ public enum PathGroup { "/provision/v2/{*}", "/zone/v2/{*}"), - /** Paths used for creating user tenants */ - onboardingUser("/application/v4/user"), - - /** Paths used for creating tenants with access control */ - onboarding("/application/v4/tenant/{ignored}"), - - /** Read-only paths used when onboarding tenants */ - onboardingInfo("/athenz/v1/", - "/athenz/v1/domains", - "/athenz/v1/properties"), - - /** Paths used for user management */ + /** Paths used for user management. */ userManagement("/user/v1/{*}"), // TODO probably add tenant and application levels. - /** Paths used by tenant administrators */ - tenantInfo("/application/v4/", // TODO move - "/application/v4/tenant/"), // TODO move + /** Paths used for creating user tenants. */ + user("/application/v4/user"), - /** Paths used by tenant administrators */ + /** Paths used for creating tenants with proper access control. */ tenant(Matcher.tenant, - "/application/v4/tenant/{tenant}", - "/application/v4/tenant/{tenant}/application/"), + "/application/v4/tenant/{tenant}"), - /** Paths used by application administrators */ + /** Paths used by tenant administrators. */ + tenantInfo(Matcher.tenant, + "/application/v4/tenant/{tenant}/application/"), + + /** Path for the base application resource. */ application(Matcher.tenant, Matcher.application, - "/application/v4/tenant/{tenant}/application/{application}", - "/application/v4/tenant/{tenant}/application/{application}/deploying/{*}", - "/application/v4/tenant/{tenant}/application/{application}/instance/{*}", - "/application/v4/tenant/{tenant}/application/{application}/environment/dev/{*}", - "/application/v4/tenant/{tenant}/application/{application}/environment/perf/{*}", - "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}/global-rotation/override"), - - /** Paths used for deployments by build service(s) */ + "/application/v4/tenant/{tenant}/application/{application}"), + + /** Paths used by application administrators. */ + applicationInfo(Matcher.tenant, + Matcher.application, + "/application/v4/tenant/{tenant}/application/{application}/deploying/{*}", + "/application/v4/tenant/{tenant}/application/{application}/instance/{*}", + "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}/logs", + "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}/suspended", + "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}/service/{*}", + "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}/global-rotation/{*}"), + + /** Path used to restart application nodes. */ // TODO move to the above when everyone is on new pipeline. + applicationRestart(Matcher.tenant, + Matcher.application, + "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{ignored}/restart"), + + /** Paths used for development deployments. */ + developmentDeployment(Matcher.tenant, + Matcher.application, + "/application/v4/tenant/{tenant}/application/{application}/environment/dev/region/{region}/instance/{instance}", + "/application/v4/tenant/{tenant}/application/{application}/environment/dev/region/{region}/instance/{instance}/deploy", + "/application/v4/tenant/{tenant}/application/{application}/environment/perf/region/{region}/instance/{instance}", + "/application/v4/tenant/{tenant}/application/{application}/environment/perf/region/{region}/instance/{instance}/deploy"), + + /** Paths used for production deployments. */ + productionDeployment(Matcher.tenant, + Matcher.application, + "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}", + "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}/deploy", + "/application/v4/tenant/{tenant}/application/{application}/environment/test/region/{region}/instance/{instance}", + "/application/v4/tenant/{tenant}/application/{application}/environment/test/region/{region}/instance/{instance}/deploy", + "/application/v4/tenant/{tenant}/application/{application}/environment/staging/region/{region}/instance/{instance}", + "/application/v4/tenant/{tenant}/application/{application}/environment/staging/region/{region}/instance/{instance}/deploy"), + + /** Paths used for continuous deployment to production. */ + submission(Matcher.tenant, + Matcher.application, + "/application/v4/tenant/{tenant}/application/{application}/submit"), + + /** Paths used for other tasks by build services. */ // TODO: This will vanish. buildService(Matcher.tenant, Matcher.application, "/application/v4/tenant/{tenant}/application/{application}/jobreport", - "/application/v4/tenant/{tenant}/application/{application}/submit", "/application/v4/tenant/{tenant}/application/{application}/promote", - "/application/v4/tenant/{tenant}/application/{application}/environment/prod/{*}", - "/application/v4/tenant/{tenant}/application/{application}/environment/test/{*}", - "/application/v4/tenant/{tenant}/application/{application}/environment/staging/{*}"), - - /** Read-only paths providing information related to deployments */ - deploymentStatus("/badge/v1/{*}", - "/zone/v1/{*}"), - - /** Paths used by some dashboard */ - dashboard("/", - "/d/{*}", - "/statuspage/v1/{*}"); + "/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/promote"), + + /** Paths which contain (not very strictly) classified information about, e.g., customers. */ + classifiedInfo("/athenz/v1/{*}", + "/cost/v1/{*}", + "/deployment/v1/{*}", + "/application/v4/", + "/application/v4/tenant/", + "/", + "/d/{*}", + "/statuspage/v1/{*}" + ), + + /** Paths providing public information. */ + publicInfo("/badge/v1/{*}", + "/zone/v1/{*}"); final List<String> pathSpecs; final List<Matcher> matchers; @@ -118,11 +143,11 @@ public enum PathGroup { public boolean matches(String path, Context context) { return get(path).map(p -> { boolean match = true; - String tenant = p.get("tenant"); + String tenant = p.get(Matcher.tenant.name); if (tenant != null && context.tenant().isPresent()) { match = context.tenant().get().value().equals(tenant); } - String application = p.get("application"); + String application = p.get(Matcher.application.name); if (application != null && context.application().isPresent()) { match &= context.application().get().value().equals(application); } @@ -138,8 +163,12 @@ public enum PathGroup { application("{application}"); final String pattern; + final String name; - Matcher(String pattern) { this.pattern = pattern; } + Matcher(String pattern) { + this.pattern = pattern; + this.name = pattern.substring(1, pattern.length() - 1); + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java index 9c09cb193da..85702ac1b89 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java @@ -1,51 +1,111 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.role; +import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.TenantName; import java.util.Set; /** * Policies for REST APIs in the controller. A policy is only considered when defined in a {@link Role}. + * A policy describes a set of {@link Privilege}s, which are valid for a set of {@link SystemName}s. + * A policy is evaluated with a {@link Context}, which provides the {@link SystemName} the policy is + * evaluated in, and any limitations to a specific {@link TenantName} or {@link ApplicationName}. * * @author mpolden */ public enum Policy { - /** Operator policy allows access to everything in all systems */ + /** Full access to everything. */ operator(Privilege.grant(Action.all()) .on(PathGroup.all()) .in(SystemName.all())), - /** - * Tenant policy allows tenants to access their own tenant, in all systems, and allows global read access in - * selected systems - */ - tenant(Privilege.grant(Action.all()) - .on(PathGroup.tenantInfo, PathGroup.tenant, PathGroup.application) - .in(SystemName.all())), - - /** Build service policy only allows access relevant for build service(s) */ - buildService(Privilege.grant(Action.all()) - .on(PathGroup.buildService) - .in(SystemName.all())), - - /** Unauthorized policy allows creation of tenants and read of everything in selected systems */ - unauthorized(Privilege.grant(Action.update) - .on(PathGroup.onboardingUser) - .in(SystemName.main, SystemName.cd, SystemName.dev), - Privilege.grant(Action.create) - .on(PathGroup.onboarding) - .in(SystemName.main, SystemName.cd, SystemName.dev), // TODO System.all() - Privilege.grant(Action.read) - .on(PathGroup.onboardingInfo) - .in(SystemName.main, SystemName.cd, SystemName.dev), - Privilege.grant(Action.read) - .on(PathGroup.all()) - .in(SystemName.main, SystemName.cd, SystemName.dev), - Privilege.grant(Action.read) - .on(PathGroup.deploymentStatus) - .in(SystemName.all())); + /** Full access to user management in select systems. */ + manager(Privilege.grant(Action.all()) + .on(PathGroup.userManagement) + .in(SystemName.Public)), + + /** Access to create a user tenant in select systems. */ + userCreate(Privilege.grant(Action.update) + .on(PathGroup.user) + .in(SystemName.main, SystemName.cd, SystemName.dev)), + + /** Access to create a tenant in select systems. */ + tenantCreate(Privilege.grant(Action.create) + .on(PathGroup.tenant) + .in(SystemName.main, SystemName.cd, SystemName.dev)), // TODO SystemName.all() + + /** Full access to tenant information and settings. */ + tenantWrite(Privilege.grant(Action.write()) + .on(PathGroup.tenant) + .in(SystemName.all())), + + /** Read access to tenant information and settings. */ + tenantRead(Privilege.grant(Action.read) + .on(PathGroup.tenant, PathGroup.tenantInfo) + .in(SystemName.all())), + + /** Access to create application under a certain tenant. */ + applicationCreate(Privilege.grant(Action.create) + .on(PathGroup.application) + .in(SystemName.all())), + + /** Read access to application information and settings. */ + applicationRead(Privilege.grant(Action.read) + .on(PathGroup.application, PathGroup.applicationInfo) + .in(SystemName.all())), + + /** Read access to application information and settings. */ + applicationUpdate(Privilege.grant(Action.update) + .on(PathGroup.application, PathGroup.applicationInfo) + .in(SystemName.all())), + + /** Access to delete a certain application. */ + applicationDelete(Privilege.grant(Action.delete) + .on(PathGroup.application) + .in(SystemName.all())), + + /** Full access to application information and settings. */ + applicationOperations(Privilege.grant(Action.write()) + .on(PathGroup.applicationInfo, PathGroup.applicationRestart) + .in(SystemName.all())), + + /** Full access to application development deployments. */ + developmentDeployment(Privilege.grant(Action.all()) + .on(PathGroup.developmentDeployment) + .in(SystemName.all())), + + /** Full access to application production deployments. */ + productionDeployment(Privilege.grant(Action.all()) + .on(PathGroup.productionDeployment) + .in(SystemName.all())), + + /** Read access to all application deployments. */ + deploymentRead(Privilege.grant(Action.read) + .on(PathGroup.developmentDeployment, PathGroup.productionDeployment) + .in(SystemName.all())), + + /** Full access to submissions for continuous deployment. */ + submission(Privilege.grant(Action.all()) + .on(PathGroup.submission) + .in(SystemName.all())), + + /** Full access to the additional tasks needed for continuous deployment. */ + deploymentPipeline(Privilege.grant(Action.all()) // TODO remove when everyone is on new pipeline. + .on(PathGroup.buildService, PathGroup.applicationRestart) + .in(SystemName.all())), + + /** Read access to all information in select systems. */ + classifiedRead(Privilege.grant(Action.read) + .on(PathGroup.all()) + .in(SystemName.main, SystemName.cd, SystemName.dev)), + + /** Read access to public info. */ + publicRead(Privilege.grant(Action.read) + .on(PathGroup.publicInfo) + .in(SystemName.all())); private final Set<Privilege> privileges; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java index 01d1c7e0eb8..cae143a92a2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java @@ -5,17 +5,70 @@ import java.util.EnumSet; import java.util.Set; /** - * This declares all tenant roles known to the controller. A role contains one or more {@link Policy}'s which decide + * This declares all tenant roles known to the controller. A role contains one or more {@link Policy}s which decide * what actions a member of a role can perform. * + * Optionally, some role definition also inherit all policies from a "lower ranking" role. Read the list of roles + * from {@code everyone} to {@code tenantAdmin}, in order, to see what policies these roles. + * * @author mpolden + * @author jonmv */ public enum Role { + /** Deus ex machina. */ hostedOperator(Policy.operator), - tenantAdmin(Policy.tenant), - tenantPipelineOperator(Policy.buildService), - everyone(Policy.unauthorized); + + /** Build service which may submit new applications for continuous deployment. */ + buildService(Policy.submission, + Policy.applicationRead), + + /** Base role which every user is part of. */ + everyone(Policy.classifiedRead, + Policy.publicRead, + Policy.userCreate, + Policy.tenantCreate), + + /** Application reader which can see all information about an application, its tenant and deployments. */ + applicationReader(everyone, + Policy.tenantRead, + Policy.applicationRead, + Policy.deploymentRead), + + /** Application developer with access to deploy to development zones. */ + applicationDeveloper(applicationReader, + Policy.developmentDeployment), + + /** Application operator with access to normal, operational tasks of an application. */ + applicationOperator(applicationDeveloper, + Policy.applicationOperations), + + /** Application administrator with full access to an already existing application, including emergency operations. */ + applicationAdmin(applicationOperator, + Policy.applicationUpdate, + Policy.productionDeployment, + Policy.submission), + + /** Tenant admin with full access to all tenant resources, including the ability to create new applications. */ + tenantAdmin(applicationAdmin, + Policy.applicationCreate, + Policy.applicationDelete, + Policy.manager, + Policy.tenantWrite), + + /** Build and continuous delivery service. */ // TODO replace with buildService, when everyone is on new pipeline. + tenantPipeline(Policy.submission, + Policy.deploymentPipeline, + Policy.productionDeployment), + + /** Tenant administrator with full access to all child resources. */ + athenzTenantAdmin(Policy.tenantWrite, + Policy.tenantRead, + Policy.applicationCreate, + Policy.applicationUpdate, + Policy.applicationDelete, + Policy.applicationOperations, + Policy.developmentDeployment); // TODO remove, as it is covered by applicationAdmin. private final Set<Policy> policies; @@ -23,6 +76,11 @@ public enum Role { this.policies = EnumSet.copyOf(Set.of(policies)); } + Role(Role inherited, Policy... policies) { + this.policies = EnumSet.copyOf(Set.of(policies)); + this.policies.addAll(inherited.policies); + } + /** * Returns whether this role is allowed to perform action in given role context. Action is allowed if at least one * policy evaluates to true. diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index c41351f12b0..1f00d99350a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -24,6 +24,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; +import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.BuildJob; @@ -534,6 +535,22 @@ public class ControllerTest { tester.deployCompletely(application, applicationPackage); } + @Test + public void testDeployApplicationWithWarnings() { + DeploymentTester tester = new DeploymentTester(); + Application application = tester.createApplication("app1", "tenant1", 1, 1L); + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .environment(Environment.prod) + .region("us-west-1") + .build(); + ZoneId zone = ZoneId.from("prod", "us-west-1"); + int warnings = 3; + tester.configServer().generateWarnings(new DeploymentId(application.id(), zone), warnings); + tester.deployCompletely(application, applicationPackage); + assertEquals(warnings, tester.applications().require(application.id()).deployments().get(zone) + .metrics().warnings().get(DeploymentMetrics.Warning.all).intValue()); + } + private void runUpgrade(DeploymentTester tester, ApplicationId application, ApplicationVersion version) { Version next = Version.fromString("6.2"); tester.upgradeSystem(next); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java index ecf849fc68c..ca8a2e34fdf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java @@ -89,10 +89,7 @@ public class InternalDeploymentTester { * Submits a new application, and returns the version of the new submission. */ public ApplicationVersion newSubmission() { - ApplicationVersion version = jobs.submit(appId, BuildJob.defaultSourceRevision, "a@b", 2, applicationPackage, new byte[0]); - tester.applicationStore().put(appId, version, applicationPackage.zippedContent()); - tester.applicationStore().put(testerId, version, new byte[0]); - return version; + return jobs.submit(appId, BuildJob.defaultSourceRevision, "a@b", 2, applicationPackage, new byte[0]); } /** diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index eaccb5fa12d..803f56fc0d7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -29,9 +29,9 @@ import com.yahoo.vespa.serviceview.bindings.ApplicationView; import com.yahoo.vespa.serviceview.bindings.ClusterView; import com.yahoo.vespa.serviceview.bindings.ServiceView; import org.apache.commons.io.IOUtils; -import org.apache.http.impl.io.EmptyInputStream; import java.io.InputStream; +import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -41,6 +41,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.logging.Level; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -57,6 +58,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer private final Version initialVersion = new Version(6, 1, 0); private final Set<DeploymentId> suspendedApplications = new HashSet<>(); private final Map<ZoneId, List<LoadBalancer>> loadBalancers = new HashMap<>(); + private final Map<DeploymentId, List<Log>> warnings = new HashMap<>(); private Version lastPrepareVersion = null; private RuntimeException prepareException = null; @@ -164,6 +166,18 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer suspendedApplications.remove(deployment); } + public void generateWarnings(DeploymentId deployment, int count) { + List<Log> logs = new ArrayList<>(count); + for (int i = 0; i < count; i++) { + Log log = new Log(); + log.time = Instant.now().toEpochMilli(); + log.level = Level.WARNING.getName(); + log.message = "log message " + (count + 1) + " generated by unit test"; + logs.add(log); + } + warnings.put(deployment, List.copyOf(logs)); + } + @Override public NodeRepositoryMock nodeRepository() { return nodeRepository; @@ -258,7 +272,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer Collections.emptyList()); setConfigChangeActions(null); prepareResponse.tenant = new TenantId("tenant"); - prepareResponse.log = Collections.emptyList(); + prepareResponse.log = warnings.getOrDefault(deployment, Collections.emptyList()); return prepareResponse; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java index e11440a372c..de9da83826d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java @@ -41,11 +41,13 @@ public class DeploymentMetricsMaintainerTest { // No metrics gathered yet assertEquals(0, app.get().metrics().queryServiceQuality(), 0); assertEquals(0, deployment.get().metrics().documentCount(), 0); + assertFalse("No timestamp set", deployment.get().metrics().instant().isPresent()); assertFalse("Never received any queries", deployment.get().activity().lastQueried().isPresent()); assertFalse("Never received any writes", deployment.get().activity().lastWritten().isPresent()); // Metrics are gathered and saved to application maintainer.maintain(); + Instant t1 = tester.clock().instant().truncatedTo(MILLIS); assertEquals(0.5, app.get().metrics().queryServiceQuality(), Double.MIN_VALUE); assertEquals(0.7, app.get().metrics().writeServiceQuality(), Double.MIN_VALUE); assertEquals(1, deployment.get().metrics().queriesPerSecond(), Double.MIN_VALUE); @@ -53,7 +55,7 @@ public class DeploymentMetricsMaintainerTest { assertEquals(3, deployment.get().metrics().documentCount(), Double.MIN_VALUE); assertEquals(4, deployment.get().metrics().queryLatencyMillis(), Double.MIN_VALUE); assertEquals(5, deployment.get().metrics().writeLatencyMillis(), Double.MIN_VALUE); - Instant t1 = tester.clock().instant().truncatedTo(MILLIS); + assertEquals(t1, deployment.get().metrics().instant().get()); assertEquals(t1, deployment.get().activity().lastQueried().get()); assertEquals(t1, deployment.get().activity().lastWritten().get()); @@ -61,6 +63,7 @@ public class DeploymentMetricsMaintainerTest { tester.clock().advance(Duration.ofHours(1)); Instant t2 = tester.clock().instant().truncatedTo(MILLIS); maintainer.maintain(); + assertEquals(t2, deployment.get().metrics().instant().get()); assertEquals(t2, deployment.get().activity().lastQueried().get()); assertEquals(t2, deployment.get().activity().lastWritten().get()); assertEquals(1, deployment.get().activity().lastQueriesPerSecond().getAsDouble(), Double.MIN_VALUE); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java index de23a675794..86370980729 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java @@ -6,12 +6,15 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.yahoo.component.Version; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.SystemName; +import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.ControllerTester; +import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.chef.ChefMock; import com.yahoo.vespa.hosted.controller.api.integration.chef.rest.PartialNodeResult; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; @@ -29,7 +32,6 @@ import java.nio.file.Paths; import java.time.Clock; import java.time.Duration; import java.time.Instant; -import java.time.ZoneId; import java.util.Map; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.component; @@ -56,7 +58,7 @@ public class MetricsReporterTest { @Test public void test_chef_metrics() { - Clock clock = Clock.fixed(Instant.ofEpochSecond(1475497913), ZoneId.systemDefault()); + Clock clock = new ManualClock(Instant.ofEpochSecond(1475497913)); ControllerTester tester = new ControllerTester(); MetricsReporter metricsReporter = createReporter(clock, tester.controller(), metrics, SystemName.cd); metricsReporter.maintain(); @@ -69,7 +71,7 @@ public class MetricsReporterTest { assertDimension(metricContext, "tenantName", "ciintegrationtests"); assertDimension(metricContext, "app", "restart.default"); assertDimension(metricContext, "zone", "prod.cd-us-east-1"); - assertEquals(727, metricEntry.getValue().get(MetricsReporter.convergeMetric).longValue()); + assertEquals(727, metricEntry.getValue().get(MetricsReporter.CONVERGENCE_METRIC).longValue()); } @Test @@ -82,7 +84,7 @@ public class MetricsReporterTest { MetricsReporter metricsReporter = createReporter(tester.controller(), metrics, SystemName.main); metricsReporter.maintain(); - assertEquals(0.0, metrics.getMetric(MetricsReporter.deploymentFailMetric)); + assertEquals(0.0, metrics.getMetric(MetricsReporter.DEPLOYMENT_FAIL_METRIC)); // Deploy all apps successfully Application app1 = tester.createApplication("app1", "tenant1", 1, 11L); @@ -95,14 +97,14 @@ public class MetricsReporterTest { tester.deployCompletely(app4, applicationPackage); metricsReporter.maintain(); - assertEquals(0.0, metrics.getMetric(MetricsReporter.deploymentFailMetric)); + assertEquals(0.0, metrics.getMetric(MetricsReporter.DEPLOYMENT_FAIL_METRIC)); // 1 app fails system-test tester.jobCompletion(component).application(app4).nextBuildNumber().uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app4, applicationPackage, false, systemTest); metricsReporter.maintain(); - assertEquals(25.0, metrics.getMetric(MetricsReporter.deploymentFailMetric)); + assertEquals(25.0, metrics.getMetric(MetricsReporter.DEPLOYMENT_FAIL_METRIC)); } @Test @@ -207,7 +209,24 @@ public class MetricsReporterTest { } @Test - public void testBuildTimeReporting() { + public void test_deployment_warnings_metric() { + DeploymentTester tester = new DeploymentTester(); + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .environment(Environment.prod) + .region("us-west-1") + .region("us-east-3") + .build(); + MetricsReporter reporter = createReporter(tester.controller(), metrics, SystemName.main); + Application application = tester.createApplication("app1", "tenant1", 1, 11L); + tester.configServer().generateWarnings(new DeploymentId(application.id(), ZoneId.from("prod", "us-west-1")), 3); + tester.configServer().generateWarnings(new DeploymentId(application.id(), ZoneId.from("prod", "us-east-3")), 4); + tester.deployCompletely(application, applicationPackage); + reporter.maintain(); + assertEquals(4, getDeploymentWarnings(application)); + } + + @Test + public void test_build_time_reporting() { InternalDeploymentTester tester = new InternalDeploymentTester(); ApplicationVersion version = tester.deployNewSubmission(); assertEquals(1000, version.buildTime().get().toEpochMilli()); @@ -215,15 +234,19 @@ public class MetricsReporterTest { MetricsReporter reporter = createReporter(tester.tester().controller(), metrics, SystemName.main); reporter.maintain(); assertEquals(tester.clock().instant().getEpochSecond() - 1, - getMetric(MetricsReporter.deploymentBuildAgeSeconds, tester.app())); + getMetric(MetricsReporter.DEPLOYMENT_BUILD_AGE_SECONDS, tester.app())); } private Duration getAverageDeploymentDuration(Application application) { - return Duration.ofSeconds(getMetric(MetricsReporter.deploymentAverageDuration, application).longValue()); + return Duration.ofSeconds(getMetric(MetricsReporter.DEPLOYMENT_AVERAGE_DURATION, application).longValue()); } private int getDeploymentsFailingUpgrade(Application application) { - return getMetric(MetricsReporter.deploymentFailingUpgrades, application).intValue(); + return getMetric(MetricsReporter.DEPLOYMENT_FAILING_UPGRADES, application).intValue(); + } + + private int getDeploymentWarnings(Application application) { + return getMetric(MetricsReporter.DEPLOYMENT_WARNINGS, application).intValue(); } private Number getMetric(String name, Application application) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index dd4558dbe2c..e0debf6d5db 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -77,7 +77,9 @@ public class ApplicationSerializerTest { deployments.add(new Deployment(zone1, applicationVersion1, Version.fromString("1.2.3"), Instant.ofEpochMilli(3))); // One deployment without cluster info and utils deployments.add(new Deployment(zone2, applicationVersion2, Version.fromString("1.2.3"), Instant.ofEpochMilli(5), createClusterUtils(3, 0.2), createClusterInfo(3, 4), - new DeploymentMetrics(2, 3, 4, 5, 6, Optional.of(Instant.now().truncatedTo(ChronoUnit.MILLIS))), + new DeploymentMetrics(2, 3, 4, 5, 6, + Optional.of(Instant.now().truncatedTo(ChronoUnit.MILLIS)), + Map.of(DeploymentMetrics.Warning.all, 3)), DeploymentActivity.create(Optional.of(activityAt), Optional.of(activityAt), OptionalDouble.of(200), OptionalDouble.of(10)))); @@ -178,6 +180,7 @@ public class ApplicationSerializerTest { assertEquals(original.deployments().get(zone2).metrics().queryLatencyMillis(), serialized.deployments().get(zone2).metrics().queryLatencyMillis(), Double.MIN_VALUE); assertEquals(original.deployments().get(zone2).metrics().writeLatencyMillis(), serialized.deployments().get(zone2).metrics().writeLatencyMillis(), Double.MIN_VALUE); assertEquals(original.deployments().get(zone2).metrics().instant(), serialized.deployments().get(zone2).metrics().instant()); + assertEquals(original.deployments().get(zone2).metrics().warnings(), serialized.deployments().get(zone2).metrics().warnings()); { // test more deployment serialization cases Application original2 = writable(original).withChange(Change.of(ApplicationVersion.from(new SourceRevision("repo1", "branch1", "commit1"), 42))).get(); Application serialized2 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original2)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 26cf9c3127e..8c28c289889 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -453,6 +453,11 @@ public class ApplicationApiTest extends ControllerContainerTest { // POST a 'restart application' command tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/default/restart", POST) + .userIdentity(USER_ID), + "Requested restart of tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/default"); + + // POST a 'restart application' command + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/default/restart", POST) .screwdriverIdentity(SCREWDRIVER_ID), "Requested restart of tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/default"); @@ -539,10 +544,28 @@ public class ApplicationApiTest extends ControllerContainerTest { "to the old pipeline, please file a ticket at yo/vespa-support and request this.\"}", 400); - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/default/job/production-us-west-1", DELETE) + // GET deployment job overview, after triggering system and staging test jobs. + assertEquals(2, tester.controller().applications().deploymentTrigger().triggerReadyJobs()); + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/default/job", GET) + .userIdentity(USER_ID), + new File("jobs.json")); + + // GET system test job overview. + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/default/job/system-test", GET) + .userIdentity(USER_ID), + new File("system-test-job.json")); + + // GET system test run 1 details. + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/default/job/system-test/run/1", GET) + .userIdentity(USER_ID), + new File("system-test-details.json")); + + // DELETE a running job to have it aborted. + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/default/job/staging-test", DELETE) .userIdentity(USER_ID), - "{\"message\":\"Nothing to abort.\"}"); + "{\"message\":\"Aborting run 1 of stagingTest for tenant1.application1\"}"); + // DELETE submission to unsubscribe from continuous deployment. tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/submit", DELETE) .userIdentity(HOSTED_VESPA_OPERATOR), "{\"message\":\"Unregistered 'tenant1.application1' from internal deployment pipeline.\"}"); @@ -1502,7 +1525,8 @@ public class ApplicationApiTest extends ControllerContainerTest { clusterInfo.put(ClusterSpec.Id.from("cluster1"), new ClusterInfo("flavor1", 37, 2, 4, 50, ClusterSpec.Type.content, hostnames)); Map<ClusterSpec.Id, ClusterUtilization> clusterUtils = new HashMap<>(); clusterUtils.put(ClusterSpec.Id.from("cluster1"), new ClusterUtilization(0.3, 0.6, 0.4, 0.3)); - DeploymentMetrics metrics = new DeploymentMetrics(1, 2, 3, 4, 5, Optional.of(Instant.ofEpochMilli(123123))); + DeploymentMetrics metrics = new DeploymentMetrics(1, 2, 3, 4, 5, + Optional.of(Instant.ofEpochMilli(123123)), Map.of()); lockedApplication = lockedApplication .withClusterInfo(deployment.zone(), clusterInfo) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json new file mode 100644 index 00000000000..1c8da68e138 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json @@ -0,0 +1,130 @@ +{ + "lastVersions": { + "platform": { + "platform": "6.1", + "at": 0, + "pending": "Waiting for current deployment to complete" + }, + "application": { + "application": { + "hash": "1.0.44-d00d", + "build": 44, + "source": { + "gitRepository": "repo", + "gitBranch": "master", + "gitCommit": "d00d" + } + }, + "at": (ignore), + "deploying": "0 of 1 complete" + } + }, + "deploying": { + "application": { + "hash": "1.0.44-d00d", + "build": 44, + "source": { + "gitRepository": "repo", + "gitBranch": "master", + "gitCommit": "d00d" + } + } + }, + "deployments": [ + {} + ], + "jobs": { + "system-test": { + "runs": [ + { + "id": 1, + "status": "running", + "start": (ignore), + "wantedPlatform": "6.1", + "wantedApplication": { + "hash": "1.0.44-d00d", + "build": 44, + "source": { + "gitRepository": "repo", + "gitBranch": "master", + "gitCommit": "d00d" + } + }, + "steps": { + "deployReal": "unfinished", + "installReal": "unfinished", + "deployTester": "unfinished", + "installTester": "unfinished", + "startTests": "unfinished", + "endTests": "unfinished", + "deactivateReal": "unfinished", + "deactivateTester": "unfinished", + "report": "unfinished" + }, + "tasks": { + "deploy": "running" + }, + "log": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/default/job/system-test/run/1" + } + ], + "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/default/job/system-test" + }, + "staging-test": { + "runs": [ + { + "id": 1, + "status": "running", + "start": (ignore), + "wantedPlatform": "6.1", + "wantedApplication": { + "hash": "1.0.44-d00d", + "build": 44, + "source": { + "gitRepository": "repo", + "gitBranch": "master", + "gitCommit": "d00d" + } + }, + "steps": { + "deployInitialReal": "unfinished", + "installInitialReal": "unfinished", + "deployReal": "unfinished", + "installReal": "unfinished", + "deployTester": "unfinished", + "installTester": "unfinished", + "startTests": "unfinished", + "endTests": "unfinished", + "deactivateReal": "unfinished", + "deactivateTester": "unfinished", + "report": "unfinished" + }, + "tasks": {}, + "log": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/default/job/staging-test/run/1" + } + ], + "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/default/job/staging-test" + }, + "us-west-1": { + "runs": [ + { + "status": "pending", + "wantedPlatform": "6.1", + "wantedApplication": { + "hash": "1.0.44-d00d", + "build": 44, + "source": { + "gitRepository": "repo", + "gitBranch": "master", + "gitCommit": "d00d" + } + }, + "tasks": { + "system-test": "running", + "staging-test": "running" + } + } + ], + "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/default/job/production-us-west-1" + } + } +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json new file mode 100644 index 00000000000..98d4e2db612 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json @@ -0,0 +1,4 @@ +{ + "active": true, + "log": {} +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-job.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-job.json new file mode 100644 index 00000000000..71f647ead11 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-job.json @@ -0,0 +1,32 @@ +{ + "1": { + "id": 1, + "status": "running", + "start": (ignore), + "wantedPlatform": "6.1", + "wantedApplication": { + "hash": "1.0.44-d00d", + "build": 44, + "source": { + "gitRepository": "repo", + "gitBranch": "master", + "gitCommit": "d00d" + } + }, + "steps": { + "deployReal": "unfinished", + "installReal": "unfinished", + "deployTester": "unfinished", + "installTester": "unfinished", + "startTests": "unfinished", + "endTests": "unfinished", + "deactivateReal": "unfinished", + "deactivateTester": "unfinished", + "report": "unfinished" + }, + "tasks": { + "deploy": "running" + }, + "log": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/default/job/system-test/run/1" + } +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolverTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolverTest.java index 5c86301d037..4628b95ad3c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolverTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolverTest.java @@ -92,29 +92,29 @@ public class AthenzRoleResolverTest { // Operators and tenant admins are tenant admins of their tenants. assertEquals(Set.of(Context.limitedTo(TENANT, tester.controller().system())), - resolver.membership(HOSTED_OPERATOR, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantAdmin)); + resolver.membership(HOSTED_OPERATOR, APPLICATION_CONTEXT_PATH).contextsFor(Role.athenzTenantAdmin)); assertEquals(emptySet(), // TODO this is wrong, but we can't do better until we ask ZMS for roles. - resolver.membership(TENANT_ADMIN, NO_CONTEXT_PATH).contextsFor(Role.tenantAdmin)); + resolver.membership(TENANT_ADMIN, NO_CONTEXT_PATH).contextsFor(Role.athenzTenantAdmin)); assertEquals(Set.of(Context.limitedTo(TENANT, tester.controller().system())), - resolver.membership(TENANT_ADMIN, TENANT_CONTEXT_PATH).contextsFor(Role.tenantAdmin)); + resolver.membership(TENANT_ADMIN, TENANT_CONTEXT_PATH).contextsFor(Role.athenzTenantAdmin)); assertEquals(emptySet(), - resolver.membership(TENANT_ADMIN, TENANT2_CONTEXT_PATH).contextsFor(Role.tenantAdmin)); + resolver.membership(TENANT_ADMIN, TENANT2_CONTEXT_PATH).contextsFor(Role.athenzTenantAdmin)); assertEquals(emptySet(), - resolver.membership(TENANT_PIPELINE, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantAdmin)); + resolver.membership(TENANT_PIPELINE, APPLICATION_CONTEXT_PATH).contextsFor(Role.athenzTenantAdmin)); assertEquals(emptySet(), - resolver.membership(USER, TENANT_CONTEXT_PATH).contextsFor(Role.tenantAdmin)); + resolver.membership(USER, TENANT_CONTEXT_PATH).contextsFor(Role.athenzTenantAdmin)); // Only build services are pipeline operators of their applications. assertEquals(emptySet(), - resolver.membership(HOSTED_OPERATOR, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantPipelineOperator)); + resolver.membership(HOSTED_OPERATOR, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantPipeline)); assertEquals(emptySet(), - resolver.membership(TENANT_ADMIN, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantPipelineOperator)); + resolver.membership(TENANT_ADMIN, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantPipeline)); assertEquals(Set.of(Context.limitedTo(TENANT, APPLICATION, tester.controller().system())), - resolver.membership(TENANT_PIPELINE, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantPipelineOperator)); + resolver.membership(TENANT_PIPELINE, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantPipeline)); assertEquals(emptySet(), - resolver.membership(TENANT_PIPELINE, APPLICATION2_CONTEXT_PATH).contextsFor(Role.tenantPipelineOperator)); + resolver.membership(TENANT_PIPELINE, APPLICATION2_CONTEXT_PATH).contextsFor(Role.tenantPipeline)); assertEquals(emptySet(), - resolver.membership(USER, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantPipelineOperator)); + resolver.membership(USER, APPLICATION_CONTEXT_PATH).contextsFor(Role.tenantPipeline)); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java index 4cf46933dcb..b4a3e674594 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java @@ -2,10 +2,12 @@ package com.yahoo.vespa.hosted.controller.role; import org.junit.Test; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Set; import java.util.regex.Pattern; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; /** @@ -15,45 +17,40 @@ import static org.junit.Assert.fail; public class PathGroupTest { @Test - public void nonOverlappingGroups() { - for (PathGroup pg : PathGroup.all()) { - for (PathGroup pg2 : PathGroup.all()) { - if (pg == pg2) continue; - Set<String> overlapping = new LinkedHashSet<>(pg.pathSpecs); - overlapping.retainAll(pg2.pathSpecs); - if ( ! overlapping.isEmpty()) { - fail("The following path specs overlap in " + pg + " and " + pg2 + ": " + overlapping); - } - } - } - } - - @Test public void uniqueMatches() { // Ensure that each path group contains at most one match for any given path, to avoid undefined context extraction. - for (PathGroup group : PathGroup.values()) - for (String path1 : group.pathSpecs) - for (String path2 : group.pathSpecs) { - if (path1 == path2) continue; + Set<String> checkedAgainstSelf = new HashSet<>(); + for (PathGroup group1 : PathGroup.values()) + for (PathGroup group2 : PathGroup.values()) + for (String path1 : group1.pathSpecs) + for (String path2 : group2.pathSpecs) { + if (path1.equals(path2)) { + if (checkedAgainstSelf.add(path1)) continue; + fail("Path '" + path1 + "' appears in both '" + group1 + "' and '" + group2 + "'."); + } - String[] parts1 = path1.split("/"); - String[] parts2 = path2.split("/"); + String[] parts1 = path1.split("/"); + String[] parts2 = path2.split("/"); - int end = Math.min(parts1.length, parts2.length); - // If one path has more parts than the other ... - // and the other doesn't end with a wildcard matcher ... - // and the longest one isn't just one wildcard longer ... - if (end < parts1.length && (end == 0 || ! parts2[end - 1].equals("{*}")) && ! parts1[end].equals("{*}")) continue; - if (end < parts2.length && (end == 0 || ! parts1[end - 1].equals("{*}")) && ! parts2[end].equals("{*}")) continue; + int end = Math.min(parts1.length, parts2.length); + // If one path has more parts than the other ... + // and the other doesn't end with a wildcard matcher ... + // and the longest one isn't just one wildcard longer ... + // then one is strictly longer than the other, and it's not a match. + if (end < parts1.length && (end == 0 || ! parts2[end - 1].equals("{*}")) && ! parts1[end].equals("{*}")) continue; + if (end < parts2.length && (end == 0 || ! parts1[end - 1].equals("{*}")) && ! parts2[end].equals("{*}")) continue; - int i; - for (i = 0; i < end; i++) - if ( ! parts1[i].equals(parts2[i]) - && ! (parts1[i].startsWith("{") && parts1[i].endsWith("}")) - && ! (parts2[i].startsWith("{") && parts2[i].endsWith("}"))) break; + int i; + for (i = 0; i < end; i++) + if ( ! parts1[i].equals(parts2[i]) + && ! (parts1[i].startsWith("{") && parts1[i].endsWith("}")) + && ! (parts2[i].startsWith("{") && parts2[i].endsWith("}"))) break; - if (i == end) fail("Paths '" + path1 + "' and '" + path2 + "' overlap."); - } + if (i == end) fail("Paths '" + path1 + "' and '" + path2 + "' overlap."); + } + + assertEquals(PathGroup.all().stream().mapToInt(group -> group.pathSpecs.size()).sum(), + checkedAgainstSelf.size()); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java index 2638e3f74e6..972ddefb1a5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java @@ -31,7 +31,7 @@ public class RoleMembershipTest { @Test public void tenant_membership() { RoleMembership roles = RoleMembership.in(SystemName.main) - .add(Role.tenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) + .add(Role.athenzTenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) .build(); assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); assertFalse("Deny access to operator API", roles.allows(Action.create, "/controller/v1/foo")); @@ -40,15 +40,15 @@ public class RoleMembershipTest { assertTrue(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); RoleMembership multiContext = RoleMembership.in(SystemName.main) - .add(Role.tenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) - .add(Role.tenantAdmin).limitedTo(TenantName.from("t2"), ApplicationName.from("a2")) + .add(Role.athenzTenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) + .add(Role.athenzTenantAdmin).limitedTo(TenantName.from("t2"), ApplicationName.from("a2")) .build(); assertFalse("Deny access to other tenant and app", multiContext.allows(Action.update, "/application/v4/tenant/t3/application/a3")); assertTrue(multiContext.allows(Action.update, "/application/v4/tenant/t2/application/a2")); assertTrue(multiContext.allows(Action.update, "/application/v4/tenant/t1/application/a1")); RoleMembership publicSystem = RoleMembership.in(SystemName.vaas) - .add(Role.tenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) + .add(Role.athenzTenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) .build(); assertFalse(publicSystem.allows(Action.read, "/controller/v1/foo")); assertTrue(multiContext.allows(Action.update, "/application/v4/tenant/t1/application/a1")); @@ -57,7 +57,7 @@ public class RoleMembershipTest { @Test public void build_service_membership() { RoleMembership roles = RoleMembership.in(SystemName.main) - .add(Role.tenantPipelineOperator).build(); + .add(Role.tenantPipeline).build(); assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); assertFalse(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); assertTrue(roles.allows(Action.create, "/application/v4/tenant/t1/application/a1/jobreport")); @@ -67,8 +67,8 @@ public class RoleMembershipTest { @Test public void multi_role_membership() { RoleMembership roles = RoleMembership.in(SystemName.main) - .add(Role.tenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) - .add(Role.tenantPipelineOperator) + .add(Role.athenzTenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) + .add(Role.tenantPipeline) .add(Role.everyone) .build(); assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); |