diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2019-10-03 12:55:34 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2019-10-03 13:40:11 +0200 |
commit | 2502468da07545cf3c867c33006e72f0880b4c20 (patch) | |
tree | 397e7205b260f204d05a8472eeaf7b24a1503b0d | |
parent | adf22d3886ccd6de163278434a1a6d502584d0f9 (diff) |
Split instance and application and creation and deletion, but let REST API do both
19 files changed, 194 insertions, 159 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/ApplicationId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/ApplicationId.java index fceecedb9fe..c2512c2032b 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/ApplicationId.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/ApplicationId.java @@ -10,10 +10,6 @@ public class ApplicationId extends NonDefaultIdentifier { super(id); } - public static boolean isLegal(String id) { - return strictPattern.matcher(id).matches(); - } - @Override public void validate() { super.validate(); @@ -21,9 +17,8 @@ public class ApplicationId extends NonDefaultIdentifier { } public static void validate(String id) { - if (!isLegal(id)) { + if ( ! strictPattern.matcher(id).matches()) throwInvalidId(id, strictPatternExplanation); - } } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/Identifier.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/Identifier.java index 2067a88e5fb..4007ac2b9cd 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/Identifier.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/Identifier.java @@ -15,7 +15,6 @@ public abstract class Identifier { protected static final String strictPatternExplanation = "New tenant or application names must start with a letter, may contain no more than 20 " + "characters, and may only contain lowercase letters, digits or dashes, but no double-dashes."; - // TODO: Use this also for instances, if they ever get proper support. protected static final Pattern strictPattern = Pattern.compile("^(?=.{1,20}$)[a-z](-?[a-z0-9]+)*$"); private static final Pattern serializedIdentifierPattern = Pattern.compile("[a-zA-Z0-9_-]+"); private static final Pattern serializedPattern = Pattern.compile("[a-zA-Z0-9_.-]+"); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/InstanceId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/InstanceId.java index 6e3087cdcf6..8e14774b827 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/InstanceId.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/InstanceId.java @@ -16,4 +16,9 @@ public class InstanceId extends SerializedIdentifier { validateNoUpperCase(); } + public static void validate(String id) { + if ( ! strictPattern.matcher(id).matches()) + throwInvalidId(id, strictPatternExplanation); + } + } 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 c012abff670..88383168229 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 @@ -7,7 +7,6 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; @@ -27,6 +26,7 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; +import com.yahoo.vespa.hosted.controller.api.identifiers.InstanceId; import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer; @@ -149,7 +149,6 @@ public class ApplicationController { // Update serialization format of all applications Once.after(Duration.ofMinutes(1), () -> { - curator.deleteOldApplicationData(); Instant start = clock.instant(); int count = 0; for (Application application : curator.readApplications()) { @@ -263,70 +262,86 @@ public class ApplicationController { * * @throws IllegalArgumentException if the application already exists */ - // TODO jonmv: split in create application and create instance - public Application createApplication(ApplicationId id, Optional<Credentials> credentials) { - if (id.instance().isTester()) - throw new IllegalArgumentException("'" + id + "' is a tester application!"); - try (Lock lock = lock(TenantAndApplicationId.from(id))) { - // Validate only application names which do not already exist. - if (getApplication(TenantAndApplicationId.from(id)).isEmpty()) - com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId.validate(id.application().value()); + public Application createApplication(TenantAndApplicationId id, Optional<Credentials> credentials) { + try (Lock lock = lock(id)) { + if (getApplication(id).isPresent()) + throw new IllegalArgumentException("Could not create '" + id + "': Application already exists"); + if (getApplication(dashToUnderscore(id)).isPresent()) // VESPA-1945 + throw new IllegalArgumentException("Could not create '" + id + "': Application " + dashToUnderscore(id) + " already exists"); + + com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId.validate(id.application().value()); Optional<Tenant> tenant = controller.tenants().get(id.tenant()); if (tenant.isEmpty()) throw new IllegalArgumentException("Could not create '" + id + "': This tenant does not exist"); - if (getInstance(id).isPresent()) - throw new IllegalArgumentException("Could not create '" + id + "': Application already exists"); - if (getInstance(dashToUnderscore(id)).isPresent()) // VESPA-1945 - throw new IllegalArgumentException("Could not create '" + id + "': Application " + dashToUnderscore(id) + " already exists"); if (tenant.get().type() != Tenant.Type.user) { if (credentials.isEmpty()) throw new IllegalArgumentException("Could not create '" + id + "': No credentials provided"); - - if ( ! id.instance().isTester()) // Only store the application permits for non-user applications. - accessControl.createApplication(id, credentials.get()); + accessControl.createApplication(id, credentials.get()); } - Application application = getApplication(TenantAndApplicationId.from(id)).orElse(new Application(TenantAndApplicationId.from(id), - clock.instant())); - LockedApplication locked = new LockedApplication(application, lock).withNewInstance(id.instance()); + + LockedApplication locked = new LockedApplication(new Application(id, clock.instant()), lock); store(locked); log.info("Created " + locked); return locked.get(); } } + /** + * Creates a new instance for an existing application. + * + * @throws IllegalArgumentException if the instance already exists, or has an invalid instance name. + */ + public void createInstance(ApplicationId id) { + if (id.instance().isTester()) + throw new IllegalArgumentException("'" + id + "' is a tester application!"); + lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { + InstanceId.validate(id.instance().value()); + + if (getInstance(id).isPresent()) + throw new IllegalArgumentException("Could not create '" + id + "': Instance already exists"); + if (getInstance(dashToUnderscore(id)).isPresent()) // VESPA-1945 + throw new IllegalArgumentException("Could not create '" + id + "': Instance " + dashToUnderscore(id) + " already exists"); + + store(application.withNewInstance(id.instance())); + log.info("Created " + id); + }); + } + public ActivateResult deploy(ApplicationId applicationId, ZoneId zone, Optional<ApplicationPackage> applicationPackageFromDeployer, DeployOptions options) { - return deploy(applicationId, zone, applicationPackageFromDeployer, Optional.empty(), options, Optional.empty()); + return deploy(applicationId, zone, applicationPackageFromDeployer, Optional.empty(), options); } /** Deploys an application. If the application does not exist it is created. */ // TODO: Get rid of the options arg - // TODO(jvenstad): Split this, and choose between deployDirectly and deploy in handler, excluding internally built from the latter. - public ActivateResult deploy(ApplicationId applicationId, ZoneId zone, + // TODO jonmv: Split this, and choose between deployDirectly and deploy in handler, excluding internally built from the latter. + public ActivateResult deploy(ApplicationId instanceId, ZoneId zone, Optional<ApplicationPackage> applicationPackageFromDeployer, Optional<ApplicationVersion> applicationVersionFromDeployer, - DeployOptions options, - Optional<Principal> deployingIdentity) { - if (applicationId.instance().isTester()) - throw new IllegalArgumentException("'" + applicationId + "' is a tester application!"); - - // TODO jonmv: Change this to create instances on demand. - Tenant tenant = controller.tenants().require(applicationId.tenant()); - if (tenant.type() == Tenant.Type.user && getInstance(applicationId).isEmpty()) + DeployOptions options) { + if (instanceId.instance().isTester()) + throw new IllegalArgumentException("'" + instanceId + "' is a tester application!"); + + TenantAndApplicationId applicationId = TenantAndApplicationId.from(instanceId); + if ( getApplication(applicationId).isEmpty() + && controller.tenants().require(instanceId.tenant()).type() == Tenant.Type.user) createApplication(applicationId, Optional.empty()); - try (Lock deploymentLock = lockForDeployment(applicationId, zone)) { + if (getInstance(instanceId).isEmpty()) + createInstance(instanceId); + + try (Lock deploymentLock = lockForDeployment(instanceId, zone)) { Version platformVersion; ApplicationVersion applicationVersion; ApplicationPackage applicationPackage; Set<ContainerEndpoint> endpoints; Optional<ApplicationCertificate> applicationCertificate; - try (Lock lock = lock(TenantAndApplicationId.from(applicationId))) { - LockedApplication application = new LockedApplication(requireApplication(TenantAndApplicationId.from(applicationId)), lock); - InstanceName instance = applicationId.instance(); + try (Lock lock = lock(applicationId)) { + LockedApplication application = new LockedApplication(requireApplication(applicationId), lock); + InstanceName instance = instanceId.instance(); boolean manuallyDeployed = options.deployDirectly || zone.environment().isManuallyDeployed(); boolean preferOldestVersion = options.deployCurrentVersion; @@ -348,25 +363,22 @@ public class ApplicationController { if ( job.isEmpty() || job.get().lastTriggered().isEmpty() || job.get().lastCompleted().isPresent() && job.get().lastCompleted().get().at().isAfter(job.get().lastTriggered().get().at())) - return unexpectedDeployment(applicationId, zone); + return unexpectedDeployment(instanceId, zone); JobRun triggered = job.get().lastTriggered().get(); platformVersion = preferOldestVersion ? triggered.sourcePlatform().orElse(triggered.platform()) : triggered.platform(); applicationVersion = preferOldestVersion ? triggered.sourceApplication().orElse(triggered.application()) : triggered.application(); - applicationPackage = getApplicationPackage(applicationId, application.get().internal(), applicationVersion); - applicationPackage = withTesterCertificate(applicationPackage, applicationId, jobType); + applicationPackage = getApplicationPackage(instanceId, application.get().internal(), applicationVersion); + applicationPackage = withTesterCertificate(applicationPackage, instanceId, jobType); validateRun(application.get(), instance, zone, platformVersion, applicationVersion); } - // TODO jonmv: Remove this when all packages are validated upon submission, as in ApplicationApiHandler.submit(...). - verifyApplicationIdentityConfiguration(applicationId.tenant(), applicationPackage, deployingIdentity); - if (zone.environment().isProduction()) // Assign and register endpoints application = withRotation(application, instance); - endpoints = registerEndpointsInDns(application.get().deploymentSpec(), application.get().require(applicationId.instance()), zone); + endpoints = registerEndpointsInDns(application.get().deploymentSpec(), application.get().require(instanceId.instance()), zone); if (controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) { @@ -386,11 +398,11 @@ public class ApplicationController { // Carry out deployment without holding the application lock. options = withVersion(platformVersion, options); - ActivateResult result = deploy(applicationId, applicationPackage, zone, options, endpoints, + ActivateResult result = deploy(instanceId, applicationPackage, zone, options, endpoints, applicationCertificate.orElse(null)); - lockApplicationOrThrow(TenantAndApplicationId.from(applicationId), application -> - store(application.with(applicationId.instance(), + lockApplicationOrThrow(applicationId, application -> + store(application.with(instanceId.instance(), instance -> instance.withNewDeployment(zone, applicationVersion, platformVersion, clock.instant(), warningsFrom(result))))); return result; @@ -703,22 +715,24 @@ public class ApplicationController { * * @throws IllegalArgumentException if the application has deployments or the caller is not authorized */ - public void deleteApplication(TenantName tenantName, ApplicationName applicationName, Optional<Credentials> credentials) { - Tenant tenant = controller.tenants().require(tenantName); + public void deleteApplication(TenantAndApplicationId id, Optional<Credentials> credentials) { + Tenant tenant = controller.tenants().require(id.tenant()); if (tenant.type() != Tenant.Type.user && credentials.isEmpty()) - throw new IllegalArgumentException("Could not delete application '" + tenantName + "." + applicationName + "': No credentials provided"); + throw new IllegalArgumentException("Could not delete application '" + id + "': No credentials provided"); // Find all instances of the application - TenantAndApplicationId id = TenantAndApplicationId.from(tenantName, applicationName); List<ApplicationId> instances = requireApplication(id).instances().keySet().stream() .map(id::instance) .collect(Collectors.toUnmodifiableList()); if (instances.size() > 1) throw new IllegalArgumentException("Could not delete application; more than one instance present: " + instances); - // TODO: Make this one transaction when database is moved to ZooKeeper for (ApplicationId instance : instances) - deleteInstance(instance, credentials); + deleteInstance(instance); + + if (tenant.type() != Tenant.Type.user) + accessControl.deleteApplication(id, credentials.get()); + curator.removeApplication(id); } /** @@ -727,24 +741,20 @@ public class ApplicationController { * @throws IllegalArgumentException if the application has deployments or the caller is not authorized * @throws NotExistsException if the instance does not exist */ - public void deleteInstance(ApplicationId applicationId, Optional<Credentials> credentials) { - Tenant tenant = controller.tenants().require(applicationId.tenant()); - if (tenant.type() != Tenant.Type.user && credentials.isEmpty()) - throw new IllegalArgumentException("Could not delete application '" + applicationId + "': No credentials provided"); - - if (getInstance(applicationId).isEmpty()) - throw new NotExistsException("Could not delete application '" + applicationId + "': Application not found"); + public void deleteInstance(ApplicationId instanceId) { + if (getInstance(instanceId).isEmpty()) + throw new NotExistsException("Could not delete application '" + instanceId + "': Application not found"); - lockApplicationOrThrow(TenantAndApplicationId.from(applicationId), application -> { - if ( ! application.get().require(applicationId.instance()).deployments().isEmpty()) + lockApplicationOrThrow(TenantAndApplicationId.from(instanceId), application -> { + if ( ! application.get().require(instanceId.instance()).deployments().isEmpty()) throw new IllegalArgumentException("Could not delete '" + application + "': It has active deployments in: " + - application.get().require(applicationId.instance()).deployments().keySet().stream().map(ZoneId::toString) + application.get().require(instanceId.instance()).deployments().keySet().stream().map(ZoneId::toString) .sorted().collect(Collectors.joining(", "))); - applicationStore.removeAll(applicationId); - applicationStore.removeAll(TesterId.of(applicationId)); + applicationStore.removeAll(instanceId); + applicationStore.removeAll(TesterId.of(instanceId)); - Instance instance = application.get().require(applicationId.instance()); + Instance instance = application.get().require(instanceId.instance()); instance.rotations().forEach(assignedRotation -> { var endpoints = instance.endpointsIn(controller.system(), assignedRotation.endpointId()); endpoints.asList().stream() @@ -753,15 +763,10 @@ public class ApplicationController { controller.nameServiceForwarder().removeRecords(Record.Type.CNAME, RecordName.from(name), Priority.normal); }); }); - curator.storeWithoutInstance(application.without(applicationId.instance()).get()); + curator.writeApplication(application.without(instanceId.instance()).get()); - log.info("Deleted " + application); + log.info("Deleted " + instanceId); }); - - - if (tenant.type() != Tenant.Type.user && getApplication(applicationId).isEmpty()) - // TODO jonmv: Implementations ignore the instance — refactor to provide tenant and application names only. - accessControl.deleteApplication(applicationId, credentials.get()); } /** @@ -846,10 +851,12 @@ public class ApplicationController { public DeploymentTrigger deploymentTrigger() { return deploymentTrigger; } + private TenantAndApplicationId dashToUnderscore(TenantAndApplicationId id) { + return TenantAndApplicationId.from(id.tenant().value(), id.application().value().replaceAll("-", "_")); + } + private ApplicationId dashToUnderscore(ApplicationId id) { - return ApplicationId.from(id.tenant().value(), - id.application().value().replaceAll("-", "_"), - id.instance().value()); + return dashToUnderscore(TenantAndApplicationId.from(id)).instance(id.instance()); } /** diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/TenantAndApplicationId.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/TenantAndApplicationId.java index 0b537535315..b4f0d6e2487 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/TenantAndApplicationId.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/TenantAndApplicationId.java @@ -48,6 +48,10 @@ public class TenantAndApplicationId implements Comparable<TenantAndApplicationId return instance(InstanceName.defaultName()); } + public ApplicationId instance(String instance) { + return instance(InstanceName.from(instance)); + } + public ApplicationId instance(InstanceName instance) { return ApplicationId.from(tenant, application, instance); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java index 91f9e2d56d7..304a47044a1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.athenz.impl; import com.google.inject.Inject; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.TenantName; import com.yahoo.log.LogLevel; @@ -15,10 +14,12 @@ import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.athenz.api.OktaAccessToken; import com.yahoo.vespa.athenz.client.zms.RoleAction; import com.yahoo.vespa.athenz.client.zms.ZmsClient; +import com.yahoo.vespa.athenz.client.zms.ZmsClientException; import com.yahoo.vespa.athenz.client.zts.ZtsClient; import com.yahoo.vespa.hosted.controller.Application; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory; import com.yahoo.vespa.hosted.controller.api.integration.athenz.ApplicationAction; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.security.AccessControl; import com.yahoo.vespa.hosted.controller.security.AthenzCredentials; import com.yahoo.vespa.hosted.controller.security.AthenzTenantSpec; @@ -142,7 +143,7 @@ public class AthenzFacade implements AccessControl { } @Override - public void createApplication(ApplicationId id, Credentials credentials) { + public void createApplication(TenantAndApplicationId id, Credentials credentials) { AthenzCredentials athenzCredentials = (AthenzCredentials) credentials; createApplication(athenzCredentials.domain(), id.application(), athenzCredentials.token()); } @@ -152,11 +153,19 @@ public class AthenzFacade implements AccessControl { log("createProviderResourceGroup(" + "tenantDomain=%s, providerDomain=%s, service=%s, resourceGroup=%s, roleActions=%s)", domain, service.getDomain().getName(), service.getName(), application, tenantRoleActions); - zmsClient.createProviderResourceGroup(domain, service, application.value(), tenantRoleActions, token); + try { + zmsClient.createProviderResourceGroup(domain, service, application.value(), tenantRoleActions, token); + } + catch (ZmsClientException e) { + if (e.getErrorCode() == com.yahoo.jdisc.Response.Status.FORBIDDEN) + throw new ForbiddenException("Not authorized to create application", e); + else + throw e; + } } @Override - public void deleteApplication(ApplicationId id, Credentials credentials) { + public void deleteApplication(TenantAndApplicationId id, Credentials credentials) { AthenzCredentials athenzCredentials = (AthenzCredentials) credentials; log("deleteProviderResourceGroup(tenantDomain=%s, providerDomain=%s, service=%s, resourceGroup=%s)", athenzCredentials.domain(), service.getDomain().getName(), service.getName(), id.application()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 0ecce359a02..54b5c339159 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -30,6 +30,7 @@ import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.persistence.BufferedLogStore; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; +import com.yahoo.vespa.hosted.controller.tenant.Tenant; import java.net.URI; import java.security.cert.X509Certificate; @@ -351,12 +352,22 @@ public class JobController { /** Stores the given package and starts a deployment of it, after aborting any such ongoing deployment. */ public void deploy(ApplicationId id, JobType type, Optional<Version> platform, ApplicationPackage applicationPackage) { + if ( ! type.environment().isManuallyDeployed()) + throw new IllegalArgumentException("Direct deployments are only allowed to manually deployed environments."); + + if ( controller.tenants().require(id.tenant()).type() == Tenant.Type.user + && controller.applications().getApplication(TenantAndApplicationId.from(id)).isEmpty()) + controller.applications().createApplication(TenantAndApplicationId.from(id), Optional.empty()); + controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { if ( ! application.get().internal()) - controller.applications().store(registered(application)); + application = registered(application); + + if ( ! application.get().instances().containsKey(id.instance())) + application = application.withNewInstance(id.instance()); + + controller.applications().store(application); }); - if ( ! type.environment().isManuallyDeployed()) - throw new IllegalArgumentException("Direct deployments are only allowed to manually deployed environments."); last(id, type).filter(run -> ! run.hasEnded()).ifPresent(run -> abortAndWait(run.id())); locked(id, type, __ -> { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 9501ac5a7f9..357dbb37b27 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -360,22 +360,11 @@ public class CuratorDb { private Stream<TenantAndApplicationId> readApplicationIds() { return curator.getChildren(applicationRoot).stream() - .filter(id -> id.split(":").length == 2) .map(TenantAndApplicationId::fromSerialized); } - public void deleteOldApplicationData() { - curator.getChildren(applicationRoot).stream() - .filter(id -> id.split(":").length == 3) - .forEach(id -> curator.delete(applicationRoot.append(id))); - } - - // TODO jonmv: Refactor when instance split operation is done - public void storeWithoutInstance(Application application) { - if (application.instances().isEmpty()) - curator.delete(applicationPath(application.id())); - else - writeApplication(application); + public void removeApplication(TenantAndApplicationId id) { + curator.delete(applicationPath(id)); } // -------------- Job Runs ------------------------------------------------ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 96d21f113a4..c1fae3bf1f5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -242,14 +242,14 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse handlePOST(Path path, HttpRequest request) { if (path.matches("/application/v4/tenant/{tenant}")) return createTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/key")) return addDeveloperKey(path.get("tenant"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return createApplication(path.get("tenant"), path.get("application"), "default", request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return createApplication(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), "default", false, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), "default", true, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), "default", request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/jobreport")) return notifyJobCompletion(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/key")) return addDeployKey(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/submit")) return submit(path.get("tenant"), path.get("application"), "default", request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}")) return createApplication(path.get("tenant"), path.get("application"), path.get("instance"), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}")) return createInstance(path.get("tenant"), path.get("application"), path.get("instance"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploy/{jobtype}")) return jobDeploy(appIdFromPath(path), jobTypeFromPath(path), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), path.get("instance"), false, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), path.get("instance"), true, request); @@ -1024,25 +1024,30 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return tenant(controller.tenants().require(TenantName.from(tenantName)), request); } - private HttpResponse createApplication(String tenantName, String applicationName, String instanceName, HttpRequest request) { + private HttpResponse createApplication(String tenantName, String applicationName, HttpRequest request) { Inspector requestObject = toSlime(request.getData()).get(); - ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName); - try { - Optional<Credentials> credentials = controller.tenants().require(id.tenant()).type() == Tenant.Type.user - ? Optional.empty() - : Optional.of(accessControlRequests.credentials(id.tenant(), requestObject, request.getJDiscRequest())); - Application application = controller.applications().createApplication(id, credentials); - - Slime slime = new Slime(); - toSlime(id, slime.setObject(), request); - return new SlimeJsonResponse(slime); - } - catch (ZmsClientException e) { // TODO: Push conversion down - if (e.getErrorCode() == com.yahoo.jdisc.Response.Status.FORBIDDEN) - throw new ForbiddenException("Not authorized to create application", e); - else - throw e; - } + TenantAndApplicationId id = TenantAndApplicationId.from(tenantName, applicationName); + Optional<Credentials> credentials = controller.tenants().require(id.tenant()).type() == Tenant.Type.user + ? Optional.empty() + : Optional.of(accessControlRequests.credentials(id.tenant(), requestObject, request.getJDiscRequest())); + Application application = controller.applications().createApplication(id, credentials); + + Slime slime = new Slime(); + toSlime(id, slime.setObject(), request); + return new SlimeJsonResponse(slime); + } + + // TODO jonmv: Remove when clients are updated. + private HttpResponse createInstance(String tenantName, String applicationName, String instanceName, HttpRequest request) { + TenantAndApplicationId applicationId = TenantAndApplicationId.from(tenantName, applicationName); + if (controller.applications().getApplication(applicationId).isEmpty()) + createApplication(tenantName, applicationName, request); + + controller.applications().createInstance(applicationId.instance(instanceName)); + + Slime slime = new Slime(); + toSlime(applicationId.instance(instanceName), slime.setObject(), request); + return new SlimeJsonResponse(slime); } /** Trigger deployment of the given Vespa version if a valid one is given, e.g., "7.8.9". */ @@ -1234,12 +1239,15 @@ public class ApplicationApiHandler extends LoggingRequestHandler { deployOptions.field("ignoreValidationErrors").asBool(), deployOptions.field("deployCurrentVersion").asBool()); + applicationPackage.ifPresent(aPackage -> controller.applications().verifyApplicationIdentityConfiguration(applicationId.tenant(), + aPackage, + Optional.of(requireUserPrincipal(request)))); + ActivateResult result = controller.applications().deploy(applicationId, zone, applicationPackage, applicationVersion, - deployOptionsJsonClass, - Optional.of(requireUserPrincipal(request))); + deployOptionsJsonClass); return new SlimeJsonResponse(toSlime(result)); } @@ -1262,22 +1270,23 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private HttpResponse deleteApplication(String tenantName, String applicationName, HttpRequest request) { - TenantName tenant = TenantName.from(tenantName); - ApplicationName application = ApplicationName.from(applicationName); - Optional<Credentials> credentials = controller.tenants().require(tenant).type() == Tenant.Type.user + TenantAndApplicationId id = TenantAndApplicationId.from(tenantName, applicationName); + Optional<Credentials> credentials = controller.tenants().require(id.tenant()).type() == Tenant.Type.user ? Optional.empty() - : Optional.of(accessControlRequests.credentials(tenant, toSlime(request.getData()).get(), request.getJDiscRequest())); - controller.applications().deleteApplication(tenant, application, credentials); - return new MessageResponse("Deleted application " + tenant + "." + application); + : Optional.of(accessControlRequests.credentials(id.tenant(), toSlime(request.getData()).get(), request.getJDiscRequest())); + controller.applications().deleteApplication(id, credentials); + return new MessageResponse("Deleted application " + id); } private HttpResponse deleteInstance(String tenantName, String applicationName, String instanceName, HttpRequest request) { - ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName); + TenantAndApplicationId id = TenantAndApplicationId.from(tenantName, applicationName); Optional<Credentials> credentials = controller.tenants().require(id.tenant()).type() == Tenant.Type.user ? Optional.empty() : Optional.of(accessControlRequests.credentials(id.tenant(), toSlime(request.getData()).get(), request.getJDiscRequest())); - controller.applications().deleteInstance(id, credentials); - return new MessageResponse("Deleted instance " + id.toFullString()); + controller.applications().deleteInstance(id.instance(instanceName)); + if (controller.applications().requireApplication(id).instances().isEmpty()) + controller.applications().deleteApplication(id, credentials); + return new MessageResponse("Deleted instance " + id.instance(instanceName).toFullString()); } private HttpResponse deactivate(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) { @@ -1469,6 +1478,15 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return Joiner.on("/").join(elements); } + private void toSlime(TenantAndApplicationId id, Cursor object, HttpRequest request) { + object.setString("tenant", id.tenant().value()); + object.setString("application", id.application().value()); + object.setString("url", withPath("/application/v4" + + "/tenant/" + id.tenant().value() + + "/application/" + id.application().value(), + request.getUri()).toString()); + } + private void toSlime(ApplicationId id, Cursor object, HttpRequest request) { object.setString("tenant", id.tenant().value()); object.setString("application", id.application().value()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index ab92e38ee4b..49015f16cce 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -64,6 +64,9 @@ import static java.util.stream.Collectors.toMap; * * @see JobController * @see ApplicationApiHandler + * + * @author smorgrav + * @author jonmv */ class JobControllerApiHandlerHelper { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AccessControl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AccessControl.java index 77ccce873fe..66c87a8eefd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AccessControl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/AccessControl.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.security; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import java.util.List; @@ -52,7 +53,7 @@ public interface AccessControl { * @param id the ID of the application to create * @param credentials the credentials for the entity requesting the creation */ - void createApplication(ApplicationId id, Credentials credentials); + void createApplication(TenantAndApplicationId id, Credentials credentials); /** * Deletes access control for the given tenant. @@ -60,7 +61,7 @@ public interface AccessControl { * @param id the ID of the application to delete * @param credentials the credentials for the entity requesting the deletion */ - void deleteApplication(ApplicationId id, Credentials credentials); + void deleteApplication(TenantAndApplicationId id, Credentials credentials); /** * Returns the list of tenants to which a user has access. diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java index 7da3e43c9a5..a88e38e5f89 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java @@ -1,10 +1,8 @@ package com.yahoo.vespa.hosted.controller.security; import com.google.inject.Inject; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.hosted.controller.Application; -import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.organization.BillingInfo; import com.yahoo.vespa.hosted.controller.api.integration.user.Roles; import com.yahoo.vespa.hosted.controller.api.integration.user.UserId; @@ -12,6 +10,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.user.UserManagement; import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole; import com.yahoo.vespa.hosted.controller.api.role.Role; import com.yahoo.vespa.hosted.controller.api.role.TenantRole; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; @@ -58,14 +57,14 @@ public class CloudAccessControl implements AccessControl { } @Override - public void createApplication(ApplicationId id, Credentials credentials) { + public void createApplication(TenantAndApplicationId id, Credentials credentials) { for (Role role : Roles.applicationRoles(id.tenant(), id.application())) userManagement.createRole(role); userManagement.addUsers(Role.applicationAdmin(id.tenant(), id.application()), List.of(new UserId(credentials.user().getName()))); } @Override - public void deleteApplication(ApplicationId id, Credentials credentials) { + public void deleteApplication(TenantAndApplicationId id, Credentials credentials) { for (ApplicationRole role : Roles.applicationRoles(id.tenant(), id.application())) userManagement.deleteRole(role); } 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 a4f3a804c55..ebf80eb9daa 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 @@ -508,7 +508,7 @@ public class ControllerTest { tester.deployAndNotify(tester.defaultInstance(app1.id()).id(), Optional.of(applicationPackage), true, systemTest); tester.applications().deactivate(app1.id().defaultInstance(), ZoneId.from(Environment.test, RegionName.from("us-east-1"))); tester.applications().deactivate(app1.id().defaultInstance(), ZoneId.from(Environment.staging, RegionName.from("us-east-3"))); - tester.applications().deleteApplication(app1.id().tenant(), app1.id().application(), tester.controllerTester().credentialsFor(app1.id())); + tester.applications().deleteApplication(app1.id(), tester.controllerTester().credentialsFor(app1.id())); try (RotationLock lock = tester.applications().rotationRepository().lock()) { assertTrue("Rotation is unassigned", tester.applications().rotationRepository().availableRotations(lock) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index cefdc3bed61..2c88d122e8f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -233,11 +233,12 @@ public final class ControllerTester { } public Application createApplication(TenantName tenant, String applicationName, String instanceName, long projectId) { - ApplicationId applicationId = ApplicationId.from(tenant.value(), applicationName, instanceName); - controller().applications().createApplication(applicationId, credentialsFor(TenantAndApplicationId.from(applicationId))); - controller().applications().lockApplicationOrThrow(TenantAndApplicationId.from(applicationId), application -> + TenantAndApplicationId applicationId = TenantAndApplicationId.from(tenant.value(), applicationName); + controller().applications().createApplication(applicationId, credentialsFor(applicationId)); + controller().applications().lockApplicationOrThrow(applicationId, application -> controller().applications().store(application.withProjectId(OptionalLong.of(projectId)))); - Application application = controller().applications().requireApplication(TenantAndApplicationId.from(applicationId)); + controller().applications().createInstance(applicationId.instance(instanceName)); + Application application = controller().applications().requireApplication(applicationId); assertTrue(application.projectId().isPresent()); return application; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java index ff245e2e488..c6bd4bde410 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java @@ -218,7 +218,7 @@ public class JobRunnerTest { // Thread is still trying to deploy tester -- delete application, and see all data is garbage collected. assertEquals(Collections.singletonList(runId), jobs.active().stream().map(run -> run.id()).collect(Collectors.toList())); - tester.controllerTester().controller().applications().deleteApplication(id.tenant(), id.application(), tester.controllerTester().credentialsFor(TenantAndApplicationId.from(id))); + tester.controllerTester().controller().applications().deleteApplication(TenantAndApplicationId.from(id), tester.controllerTester().credentialsFor(TenantAndApplicationId.from(id))); assertEquals(Collections.emptyList(), jobs.active()); assertEquals(runId, jobs.last(id, systemTest).get().id()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java index 2d8c937097a..80e52f373d7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java @@ -79,8 +79,10 @@ public class ContainerControllerTester { Optional.of(new PropertyId("1234"))); controller().tenants().create(tenantSpec, credentials); - ApplicationId app = ApplicationId.from(tenant, application, instance); - return controller().applications().createApplication(app, Optional.of(credentials)); + TenantAndApplicationId id = TenantAndApplicationId.from(tenant, application); + controller().applications().createApplication(id, Optional.of(credentials)); + controller().applications().createInstance(id.instance(instance)); + return controller().applications().requireApplication(id); } public void deploy(ApplicationId id, ApplicationPackage applicationPackage, ZoneId zone) { 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 0ae077f3fd7..918b193eb02 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 @@ -313,7 +313,7 @@ public class ApplicationApiTest extends ControllerContainerTest { .region("us-west-1") .build(); - tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2", POST) + tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2/instance/default", POST) .userIdentity(USER_ID) .oktaAccessToken(OKTA_AT), new File("application-reference-2.json")); @@ -1048,7 +1048,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1", POST) .oktaAccessToken(OKTA_AT) .userIdentity(USER_ID), - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not create 'tenant1.application1.instance1': Application already exists\"}", + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not create 'tenant1.application1.instance1': Instance already exists\"}", 400); ConfigServerMock configServer = serviceRegistry().configServerMock(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/application-created.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/application-created.json index 6cf4dc76173..31bdb07b26b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/application-created.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/application-created.json @@ -1,6 +1,5 @@ { "tenant": "my-tenant", "application": "my-app", - "instance": "default", - "url": "http://localhost:8080/application/v4/tenant/my-tenant/application/my-app/instance/default" + "url": "http://localhost:8080/application/v4/tenant/my-tenant/application/my-app" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-with-keys.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-with-keys.json index f42534a4009..b7970a48963 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-with-keys.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-with-keys.json @@ -10,12 +10,5 @@ "key": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFELzPyinTfQ/sZnTmRp5E4Ve/sbE\npDhJeqczkyFcT2PysJ5sZwm7rKPEeXDOhzTPCyRvbUqc2SGdWbKUGGa/Yw==\n-----END PUBLIC KEY-----\n", "user": "operator@tenant" }], - "applications": [ - { - "tenant":"my-tenant", - "application":"my-app", - "instance":"default", - "url":"http://localhost:8080/application/v4/tenant/my-tenant/application/my-app/instance/default" - } - ] + "applications": [] } |