Merge pull request #8717 from vespa-engine/mpolden/audit-log-pinning

Log pin actions to audit log

4 files changed, 18 insertions, 12 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java
index 55bad0b77ac..7fbd1a955a1 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java
@@ -17,8 +17,6 @@ public class MockContactRetriever extends AbstractComponent implements ContactRe
 
     private final Map<PropertyId, Contact> contacts = new HashMap<>();
 
-
-
     @Override
     public Contact getContact(Optional<PropertyId> propertyId) {
         return contacts.getOrDefault(propertyId.get(), contact());
@@ -28,7 +26,6 @@ public class MockContactRetriever extends AbstractComponent implements ContactRe
         contacts.put(propertyId, contact);
     }
 
-
     public Contact contact() {
         return new Contact(URI.create("contacts.tld"), URI.create("properties.tld"), URI.create("issues.tld"), Collections.emptyList(), "queue", Optional.of("component"));
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java
index 8bffa455b7e..a02f28e371d 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java
@@ -36,10 +36,13 @@ public class ContactInformationMaintainer extends Maintainer {
     @Override
     protected void maintain() {
         for (Tenant tenant : controller().tenants().asList()) {
-            try{
-            Optional<PropertyId> tenantPropertyId = tenant instanceof AthenzTenant ? ((AthenzTenant) tenant).propertyId() : Optional.empty();
-            Contact contact = contactRetriever.getContact(tenantPropertyId);
-            controller().tenants().lockIfPresent(tenant.name(), lockedTenant -> controller().tenants().store(lockedTenant.with(contact)));
+            try {
+                Optional<PropertyId> tenantPropertyId = Optional.empty();
+                if (tenant instanceof AthenzTenant) {
+                    tenantPropertyId = ((AthenzTenant) tenant).propertyId();
+                }
+                Contact contact = contactRetriever.getContact(tenantPropertyId);
+                controller().tenants().lockIfPresent(tenant.name(), lockedTenant -> controller().tenants().store(lockedTenant.with(contact)));
             } catch (Exception e) {
                 log.log(LogLevel.WARNING, "Failed to update contact information for " + tenant + ": " +
                                           Exceptions.toMessageString(e) + ". Retrying in " +
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 6b5ffc23f0a..6fc5cc645e8 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
@@ -206,9 +206,9 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         if (path.matches("/application/v4/tenant/{tenant}")) return createTenant(path.get("tenant"), 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}/promote")) return promoteApplication(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"), readToString(request.getData()), false);
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), readToString(request.getData()), true);
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"));
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), false, request);
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), true, request);
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), 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}/submit")) return submit(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return trigger(appIdFromPath(path), jobTypeFromPath(path), request);
@@ -803,7 +803,9 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     }
 
     /** Trigger deployment of the given Vespa version if a valid one is given, e.g., "7.8.9". */
-    private HttpResponse deployPlatform(String tenantName, String applicationName, String versionString, boolean pin) {
+    private HttpResponse deployPlatform(String tenantName, String applicationName, boolean pin, HttpRequest request) {
+        request = controller.auditLogger().log(request);
+        String versionString = readToString(request.getData());
         ApplicationId id = ApplicationId.from(tenantName, applicationName, "default");
         StringBuilder response = new StringBuilder();
         controller.applications().lockOrThrow(id, application -> {
@@ -829,7 +831,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     }
 
     /** Trigger deployment to the last known application package for the given application. */
-    private HttpResponse deployApplication(String tenantName, String applicationName) {
+    private HttpResponse deployApplication(String tenantName, String applicationName, HttpRequest request) {
+        controller.auditLogger().log(request);
         ApplicationId id = ApplicationId.from(tenantName, applicationName, "default");
         StringBuilder response = new StringBuilder();
         controller.applications().lockOrThrow(id, application -> {
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 2cdc124a88c..c0c2d4043d9 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
@@ -400,6 +400,9 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       .userIdentity(USER_ID)
                                       .data("6.1.0"),
                               "{\"message\":\"Triggered pin to 6.1 for tenant1.application1\"}");
+        assertTrue("Action is logged to audit log",
+                   tester.controller().auditLogger().readLog().entries().stream()
+                         .anyMatch(entry -> entry.resource().equals("/application/v4/tenant/tenant1/application/application1/deploying/pin")));
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying", GET)
                                       .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true}");
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying/pin", GET)