Merge pull request #21036 from vespa-engine/bjorncs/archive-key-policy

Only update policy for a key once

2 files changed, 20 insertions, 3 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainer.java
index 8156a2e9f3b..e3f69f59d24 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainer.java
@@ -6,6 +6,7 @@ import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Controller;
+import com.yahoo.vespa.hosted.controller.api.integration.archive.ArchiveBucket;
 import com.yahoo.vespa.hosted.controller.api.integration.archive.ArchiveService;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 import com.yahoo.vespa.hosted.controller.archive.CuratorArchiveBucketDb;
@@ -15,8 +16,11 @@ import com.yahoo.vespa.hosted.controller.tenant.Tenant;
 import java.time.Duration;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 
+import static java.util.stream.Collectors.groupingBy;
+
 /**
  * Update archive access permissions with roles from tenants
  *
@@ -52,10 +56,20 @@ public class ArchiveAccessMaintainer extends ControllerMaintainer {
                     try {
                         var tenantArchiveAccessRoles = cloudTenantArchiveExternalAccessRoles();
                         archiveBucketDb.buckets(zoneId).forEach(archiveBucket ->
-                                archiveService.updateBucketAndKeyPolicy(zoneId, archiveBucket,
+                                archiveService.updateBucketPolicy(zoneId, archiveBucket,
                                         Maps.filterEntries(tenantArchiveAccessRoles,
                                                 entry -> archiveBucket.tenants().contains(entry.getKey())))
                         );
+                        Map<String, List<ArchiveBucket>> bucketsPerKey = archiveBucketDb.buckets(zoneId).stream()
+                                .collect(groupingBy(ArchiveBucket::keyArn));
+                        bucketsPerKey.forEach((keyArn, buckets) -> {
+                            Set<String> authorizedIamRolesForKey = buckets.stream()
+                                    .flatMap(b -> b.tenants().stream())
+                                    .filter(tenantArchiveAccessRoles::containsKey)
+                                    .map(tenantArchiveAccessRoles::get)
+                                    .collect(Collectors.toSet());
+                            archiveService.updateKeyPolicy(zoneId, keyArn, authorizedIamRolesForKey);
+                        });
                     } catch (Exception e) {
                         throw new RuntimeException("Failed to maintain archive access in " + zoneId.value(), e);
                     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainerTest.java
index fe8dc0b1e29..df2b462914e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainerTest.java
@@ -15,6 +15,7 @@ import org.junit.Test;
 import java.time.Duration;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
@@ -39,10 +40,12 @@ public class ArchiveAccessMaintainerTest {
         var testBucket = new ArchiveBucket("bucketName", "keyArn").withTenant(tenant1);
 
         MockArchiveService archiveService = (MockArchiveService) tester.controller().serviceRegistry().archiveService();
-        assertNull(archiveService.authorizedIamRoles.get(testBucket));
+        assertNull(archiveService.authorizedIamRolesForBucket.get(testBucket));
+        assertNull(archiveService.authorizedIamRolesForKey.get(testBucket.keyArn()));
         MockMetric metric = new MockMetric();
         new ArchiveAccessMaintainer(tester.controller(), metric, Duration.ofMinutes(10)).maintain();
-        assertEquals(Map.of(tenant1, tenant1role), archiveService.authorizedIamRoles.get(testBucket));
+        assertEquals(Map.of(tenant1, tenant1role), archiveService.authorizedIamRolesForBucket.get(testBucket));
+        assertEquals(Set.of(tenant1role), archiveService.authorizedIamRolesForKey.get(testBucket.keyArn()));
 
         var expected = Map.of("archive.bucketCount",
                 tester.controller().zoneRegistry().zones().all().ids().stream()