Merge branch 'master' into smorgrav/cost_npe

8 files changed, 45 insertions, 39 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java
index 4f61c811a9b..a4bddf86cbb 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java
@@ -140,7 +140,7 @@ public class TenantController {
             if (updatedTenant.isAthensTenant() && ! token.isPresent())
                 throw new IllegalArgumentException("Could not update " + updatedTenant + ": No NToken provided");
 
-            updateAthensDomain(updatedTenant, token);
+            updateAthenzDomain(updatedTenant, token);
             db.updateTenant(updatedTenant);
             log.info("Updated " + updatedTenant);
         } catch (PersistenceException e) {
@@ -148,7 +148,7 @@ public class TenantController {
         }
     }
 
-    private void updateAthensDomain(Tenant updatedTenant, Optional<NToken> token) {
+    private void updateAthenzDomain(Tenant updatedTenant, Optional<NToken> token) {
         Tenant existingTenant = tenant(updatedTenant.getId()).get();
         if ( ! existingTenant.isAthensTenant()) return;
 
@@ -192,7 +192,7 @@ public class TenantController {
         }
     }
 
-    public Tenant migrateTenantToAthens(TenantId tenantId,
+    public Tenant migrateTenantToAthenz(TenantId tenantId,
                                         AthenzDomain tenantDomain,
                                         PropertyId propertyId,
                                         Property property,
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/ZmsClientImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/ZmsClientImpl.java
index cf2f7c798c6..110e06b767c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/ZmsClientImpl.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/ZmsClientImpl.java
@@ -121,7 +121,7 @@ public class ZmsClientImpl implements ZmsClient {
                     DomainList domainList = zmsClient.getDomainList(
                             /*limit*/null, /*skip*/null, prefix, /*depth*/null, /*domain*/null,
                             /*productId*/ null, /*modifiedSince*/null);
-                    return toAthensDomains(domainList.getNames());
+                    return toAthenzDomains(domainList.getNames());
                 });
     }
 
@@ -139,7 +139,7 @@ public class ZmsClientImpl implements ZmsClient {
         log("getServiceIdentity(domain=%s, service=%s)", service.getDomain().id(), service.getServiceName());
         return getOrThrow(() -> {
             ServiceIdentity serviceIdentity = zmsClient.getServiceIdentity(service.getDomain().id(), service.getServiceName());
-            return toAthensPublicKeys(serviceIdentity.getPublicKeys());
+            return toAthenzPublicKeys(serviceIdentity.getPublicKeys());
         });
     }
 
@@ -153,11 +153,11 @@ public class ZmsClientImpl implements ZmsClient {
                 .collect(toList());
     }
 
-    private static List<AthenzDomain> toAthensDomains(List<String> domains) {
+    private static List<AthenzDomain> toAthenzDomains(List<String> domains) {
         return domains.stream().map(AthenzDomain::new).collect(toList());
     }
 
-    private static List<AthenzPublicKey> toAthensPublicKeys(List<PublicKeyEntry> publicKeys) {
+    private static List<AthenzPublicKey> toAthenzPublicKeys(List<PublicKeyEntry> publicKeys) {
         return publicKeys.stream()
                 .map(entry -> fromYbase64EncodedKey(entry.getKey(), entry.getId()))
                 .collect(toList());
@@ -192,7 +192,7 @@ public class ZmsClientImpl implements ZmsClient {
     }
 
     private static void logWarning(ZMSClientException e) {
-        log.warning("Error from Athens: " + e.getMessage());
+        log.warning("Error from Athenz: " + e.getMessage());
     }
 
     private String resourceStringPrefix(AthenzDomain tenantDomain) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java
index c807a7f0586..baf60b612b0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java
@@ -18,6 +18,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -30,6 +31,8 @@ import java.util.stream.Collectors;
  */
 public class ClusterInfoMaintainer extends Maintainer {
 
+    private static final Logger log = Logger.getLogger(ClusterInfoMaintainer.class.getName());
+    
     private final Controller controller;
 
     ClusterInfoMaintainer(Controller controller, Duration duration, JobControl jobControl) {
@@ -53,7 +56,7 @@ public class ClusterInfoMaintainer extends Maintainer {
         for (String id : clusters.keySet()) {
             List<NodeList.Node> clusterNodes = clusters.get(id);
 
-            //Assume they are all equal and use first node as a representatitve for the cluster
+            // Assume they are all equal and use first node as a representative for the cluster
             NodeList.Node node = clusterNodes.get(0);
 
             // Extract flavor info
@@ -73,7 +76,7 @@ public class ClusterInfoMaintainer extends Maintainer {
             // Add to map
             List<String> hostnames = clusterNodes.stream().map(node1 -> node1.hostname).collect(Collectors.toList());
             ClusterInfo inf = new ClusterInfo(node.flavor, node.cost, cpu, mem, disk,
-                    ClusterSpec.Type.from(node.membership.clusterType), hostnames);
+                                              ClusterSpec.Type.from(node.membership.clusterType), hostnames);
             infoMap.put(new ClusterSpec.Id(id), inf);
         }
 
@@ -82,7 +85,6 @@ public class ClusterInfoMaintainer extends Maintainer {
 
     @Override
     protected void maintain() {
-
         for (Application application : controller().applications().asList()) {
             try (Lock lock = controller().applications().lock(application.id())) {
                 for (Deployment deployment : application.deployments().values()) {
@@ -92,11 +94,13 @@ public class ClusterInfoMaintainer extends Maintainer {
                         Map<ClusterSpec.Id, ClusterInfo> clusterInfo = getClusterInfo(nodes, deployment.zone());
                         Application app = application.with(deployment.withClusterInfo(clusterInfo));
                         controller.applications().store(app, lock);
-                    } catch (IOException ioe) {
-                        Logger.getLogger(ClusterInfoMaintainer.class.getName()).fine(ioe.getMessage());
+                    } 
+                    catch (IOException | IllegalArgumentException e) {
+                        log.log(Level.WARNING, "Failing getting cluster info of for " + deploymentId, e);
                     }
                 }
             }
         }
     }
+
 }
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 99530557981..c50f1464be7 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
@@ -160,7 +160,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         if (path.matches("/application/v4/user")) return authenticatedUser(request);
         if (path.matches("/application/v4/tenant")) return tenants(request);
         if (path.matches("/application/v4/tenant-pipeline")) return tenantPipelines();
-        if (path.matches("/application/v4/athensDomain")) return athensDomains(request);
+        if (path.matches("/application/v4/athensDomain")) return athenzDomains(request);
         if (path.matches("/application/v4/property")) return properties();
         if (path.matches("/application/v4/cookiefreshness")) return cookieFreshness(request);
         if (path.matches("/application/v4/tenant/{tenant}")) return tenant(path.get("tenant"), request);
@@ -269,12 +269,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return new SlimeJsonResponse(slime);
     }
     
-    private HttpResponse athensDomains(HttpRequest request) {
+    private HttpResponse athenzDomains(HttpRequest request) {
         Slime slime = new Slime();
         Cursor response = slime.setObject();
         Cursor array = response.setArray("data");
-        for (AthenzDomain athensDomain : controller.getDomainList(request.getProperty("prefix"))) {
-            array.addString(athensDomain.id());
+        for (AthenzDomain athenzDomain : controller.getDomainList(request.getProperty("prefix"))) {
+            array.addString(athenzDomain.id());
         }
         return new SlimeJsonResponse(slime);
     }
@@ -638,7 +638,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         if (tenant.isOpsDbTenant())
             throwIfNotSuperUserOrPartOfOpsDbGroup(new UserGroup(mandatory("userGroup", requestData).asString()), request);
         if (tenant.isAthensTenant())
-            throwIfNotAthensDomainAdmin(new AthenzDomain(mandatory("athensDomain", requestData).asString()), request);
+            throwIfNotAthenzDomainAdmin(new AthenzDomain(mandatory("athensDomain", requestData).asString()), request);
  
         controller.tenants().addTenant(tenant, authorizer.getNToken(request));
         return new SlimeJsonResponse(toSlime(tenant, request, true));
@@ -652,11 +652,11 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         PropertyId propertyId = new PropertyId(mandatory("propertyId", requestData).asString());
 
         authorizer.throwIfUnauthorized(tenantid, request);
-        throwIfNotAthensDomainAdmin(tenantDomain, request);
+        throwIfNotAthenzDomainAdmin(tenantDomain, request);
         NToken nToken = authorizer.getNToken(request)
                 .orElseThrow(() ->
                         new BadRequestException("The NToken for a domain admin is required to migrate tenant to Athens"));
-        Tenant tenant = controller.tenants().migrateTenantToAthens(tenantid, tenantDomain, propertyId, property, nToken);
+        Tenant tenant = controller.tenants().migrateTenantToAthenz(tenantid, tenantDomain, propertyId, property, nToken);
         return new SlimeJsonResponse(toSlime(tenant, request, true));
     }
 
@@ -769,6 +769,9 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
                                                           tenant,
                                                           applicationId);
         } else { // In case of host-based principal
+            // TODO What about other user type principals like Bouncer?
+            log.log(LogLevel.WARNING,
+                    "Using deprecated DeployAuthorizer.throwIfUnauthorizedForDeploy. Principal=" + principal);
             UserId userId = new UserId(principal.getName());
             deployAuthorizer.throwIfUnauthorizedForDeploy(
                     Environment.from(environment),
@@ -959,11 +962,11 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         }
     }
 
-    private void throwIfNotAthensDomainAdmin(AthenzDomain tenantDomain, HttpRequest request) {
+    private void throwIfNotAthenzDomainAdmin(AthenzDomain tenantDomain, HttpRequest request) {
         UserId userId = authorizer.getUserId(request);
-        if ( ! authorizer.isAthensDomainAdmin(userId, tenantDomain)) {
+        if ( ! authorizer.isAthenzDomainAdmin(userId, tenantDomain)) {
             throw new ForbiddenException(
-                    String.format("The user '%s' is not admin in Athens domain '%s'", userId.id(), tenantDomain.id()));
+                    String.format("The user '%s' is not admin in Athenz domain '%s'", userId.id(), tenantDomain.id()));
         }
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
index cbd39b201c1..93dc2541385 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
@@ -92,7 +92,7 @@ public class Authorizer {
     }
 
     public boolean isSuperUser(HttpRequest request) {
-        // TODO Check membership of admin role in Vespa's Athens domain
+        // TODO Check membership of admin role in Vespa's Athenz domain
         return isMemberOfVespaBouncerGroup(request) || isScrewdriverPrincipal(getPrincipal(request));
     }
 
@@ -114,7 +114,7 @@ public class Authorizer {
     private boolean isTenantAdmin(UserId userId, Tenant tenant) {
         switch (tenant.tenantType()) {
             case ATHENS:
-                return isAthensTenantAdmin(userId, tenant.getAthensDomain().get());
+                return isAthenzTenantAdmin(userId, tenant.getAthensDomain().get());
             case OPSDB:
                 return isGroupMember(userId, tenant.getUserGroup().get());
             case USER:
@@ -123,12 +123,12 @@ public class Authorizer {
         throw new IllegalArgumentException("Unknown tenant type: " + tenant.tenantType());
     }
 
-    private boolean isAthensTenantAdmin(UserId userId, AthenzDomain tenantDomain) {
+    private boolean isAthenzTenantAdmin(UserId userId, AthenzDomain tenantDomain) {
         return athenzClientFactory.createZmsClientWithServicePrincipal()
                 .hasTenantAdminAccess(AthenzUtils.createPrincipal(userId), tenantDomain);
     }
 
-    public boolean isAthensDomainAdmin(UserId userId, AthenzDomain tenantDomain) {
+    public boolean isAthenzDomainAdmin(UserId userId, AthenzDomain tenantDomain) {
         return athenzClientFactory.createZmsClientWithServicePrincipal()
                 .isDomainAdmin(AthenzUtils.createPrincipal(userId), tenantDomain);
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java
index 209f17464a7..7cf19629774 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java
@@ -43,12 +43,12 @@ public class DeployAuthorizer {
                                              Environment environment,
                                              Tenant tenant,
                                              ApplicationId applicationId) {
-        if (athensCredentialsRequired(environment, tenant, applicationId, principal))
-            checkAthensCredentials(principal, tenant, applicationId);
+        if (athenzCredentialsRequired(environment, tenant, applicationId, principal))
+            checkAthenzCredentials(principal, tenant, applicationId);
     }
 
     // TODO: inline when deployment via ssh is removed
-    private boolean athensCredentialsRequired(Environment environment, Tenant tenant, ApplicationId applicationId, Principal principal) {
+    private boolean athenzCredentialsRequired(Environment environment, Tenant tenant, ApplicationId applicationId, Principal principal) {
         if (!environmentRequiresAuthorization(environment))  return false;
 
         if (! isScrewdriverPrincipal(principal))
@@ -61,13 +61,13 @@ public class DeployAuthorizer {
 
 
     // TODO: inline when deployment via ssh is removed
-    private void checkAthensCredentials(Principal principal, Tenant tenant, ApplicationId applicationId) {
+    private void checkAthenzCredentials(Principal principal, Tenant tenant, ApplicationId applicationId) {
         AthenzDomain domain = tenant.getAthensDomain().get();
         if (! (principal instanceof AthenzPrincipal))
             throw loggedForbiddenException("Principal '%s' is not authenticated.", principal.getName());
 
         AthenzPrincipal athensPrincipal = (AthenzPrincipal)principal;
-        if ( ! hasDeployAccessToAthensApplication(athensPrincipal, domain, applicationId))
+        if ( ! hasDeployAccessToAthenzApplication(athensPrincipal, domain, applicationId))
             throw loggedForbiddenException(
                     "Screwdriver principal '%1$s' does not have deploy access to '%2$s'. " +
                     "Either the application has not been created at " + zoneRegistry.getDashboardUri() + " or " +
@@ -90,18 +90,17 @@ public class DeployAuthorizer {
                                              Tenant tenant,
                                              ApplicationId applicationId,
                                              Optional<ScrewdriverId> optionalScrewdriverId) {
-
         Principal principal = new UnauthenticatedUserPrincipal(userId.id());
 
-        if (athensCredentialsRequired(environment, tenant, applicationId, principal)) {
+        if (athenzCredentialsRequired(environment, tenant, applicationId, principal)) {
             ScrewdriverId screwdriverId = optionalScrewdriverId.orElseThrow(
                     () -> loggedForbiddenException("Screwdriver id must be provided when deploying from Screwdriver."));
             principal = AthenzUtils.createPrincipal(screwdriverId);
-            checkAthensCredentials(principal, tenant, applicationId);
+            checkAthenzCredentials(principal, tenant, applicationId);
         }
     }
 
-    private boolean hasDeployAccessToAthensApplication(AthenzPrincipal principal, AthenzDomain domain, ApplicationId applicationId) {
+    private boolean hasDeployAccessToAthenzApplication(AthenzPrincipal principal, AthenzDomain domain, ApplicationId applicationId) {
         try {
             return athenzClientFactory.createZmsClientWithServicePrincipal()
                     .hasApplicationAccess(
@@ -111,7 +110,7 @@ public class DeployAuthorizer {
                             new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(applicationId.application().value()));
         } catch (ZmsException e) {
             throw loggedForbiddenException(
-                    "Failed to authorize deployment through Athens. If this problem persists, " +
+                    "Failed to authorize deployment through Athenz. If this problem persists, " +
                             "please create ticket at yo/vespa-support. (" + e.getMessage() + ")");
         }
     }
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 d39f72ec1b8..e36645175a7 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
@@ -381,7 +381,7 @@ public class ControllerTest {
 
         // Migrate tenant to Athens
         NToken nToken = TestIdentities.userNToken;
-        tester.controller().tenants().migrateTenantToAthens(
+        tester.controller().tenants().migrateTenantToAthenz(
                 tenantId, athensDomain, new PropertyId("1567"), new Property("vespa_dev.no"), nToken);
 
         // Verify that tenant is migrated
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 ef8a3809b25..1ac5dfeb58a 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
@@ -529,7 +529,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       "{\"athensDomain\":\"domain1\", \"property\":\"property1\"}",
                                       Request.Method.POST,
                                       "domain1", unauthorizedUser),
-                              "{\"error-code\":\"FORBIDDEN\",\"message\":\"The user 'othertenant' is not admin in Athens domain 'domain1'\"}",
+                              "{\"error-code\":\"FORBIDDEN\",\"message\":\"The user 'othertenant' is not admin in Athenz domain 'domain1'\"}",
                               403);
 
         // (Create it with the right tenant id)