diff options
author | Håkon Hallingstad <hakon@yahooinc.com> | 2023-07-31 13:10:29 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@yahooinc.com> | 2023-07-31 13:10:29 +0200 |
commit | 1323447cb1ad553fe50d822fdc08385b86510e36 (patch) | |
tree | b9708454ddbcbcfd57b423ab5db9bcd186d781e4 | |
parent | d488a7482e93ae233be571d61946caa796aba588 (diff) |
simpler-acl has rolled out
6 files changed, 17 insertions, 25 deletions
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 326e8f2dcae..d1f5ba6295e 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -58,12 +58,6 @@ public class Flags { // if any, or otherwise hosted-vespa:tenant-host:default. APPLICATION_ID, TENANT_ID, CLUSTER_ID, CLUSTER_TYPE); - public static final UnboundBooleanFlag SIMPLER_ACL = defineFeatureFlag( - "simpler-acl", true, - List.of("hakonhall"), "2023-07-04", "2023-08-04", - "Simplify ACL in hosted Vespa", - "Takes effect on the next fetch of ACL rules"); - public static final UnboundDoubleFlag DEFAULT_TERM_WISE_LIMIT = defineDoubleFlag( "default-term-wise-limit", 1.0, List.of("baldersheim"), "2020-12-02", "2023-12-31", diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index a80f07acba2..864566f119e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -616,8 +616,8 @@ public final class Node implements Nodelike { } /** Returns the ACL for the node (trusted nodes, networks and ports) */ - public NodeAcl acl(NodeList allNodes, LoadBalancers loadBalancers, Zone zone, boolean simplerAcl) { - return NodeAcl.from(this, allNodes, loadBalancers, zone, simplerAcl); + public NodeAcl acl(NodeList allNodes, LoadBalancers loadBalancers, Zone zone) { + return NodeAcl.from(this, allNodes, loadBalancers, zone); } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 13a6c35e9a7..602314bed96 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -220,11 +220,11 @@ public class NodeRepository extends AbstractComponent { * @param host node for which to generate ACLs * @return the list of node ACLs */ - public List<NodeAcl> getChildAcls(Node host, boolean simplerAcl) { + public List<NodeAcl> getChildAcls(Node host) { if ( ! host.type().isHost()) throw new IllegalArgumentException("Only hosts have children"); NodeList allNodes = nodes().list(); return allNodes.childrenOf(host) - .mapToList(childNode -> childNode.acl(allNodes, loadBalancers, zone, simplerAcl)); + .mapToList(childNode -> childNode.acl(allNodes, loadBalancers, zone)); } /** Removes this application: all nodes are set dirty. */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java index 7df19c97659..d0e72cea8fc 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java @@ -45,12 +45,12 @@ public record NodeAcl(Node node, this.trustedUdpPorts = ImmutableSet.copyOf(Objects.requireNonNull(trustedUdpPorts, "trustedUdpPorts must be non-null")); } - public static NodeAcl from(Node node, NodeList allNodes, LoadBalancers loadBalancers, Zone zone, boolean simplerAcl) { + public static NodeAcl from(Node node, NodeList allNodes, LoadBalancers loadBalancers, Zone zone) { Set<TrustedNode> trustedNodes = new TreeSet<>(Comparator.comparing(TrustedNode::hostname)); Set<Integer> trustedPorts = new LinkedHashSet<>(); Set<Integer> trustedUdpPorts = new LinkedHashSet<>(); Set<String> trustedNetworks = new LinkedHashSet<>(); - IP.Space ipSpace = simplerAcl ? IP.Space.of(zone, node.cloudAccount()) : (ip, account) -> true; + IP.Space ipSpace = IP.Space.of(zone, node.cloudAccount()); // For all cases below, trust: // - SSH: If the host has one container, and it is using the host's network namespace, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeAclResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeAclResponse.java index 784f8f82d14..6fe14715355 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeAclResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeAclResponse.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.provision.restapi; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.NodeAcl; @@ -34,9 +33,8 @@ public class NodeAclResponse extends SlimeJsonResponse { Node node = nodeRepository.nodes().node(hostname) .orElseThrow(() -> new NotFoundException("No node with hostname '" + hostname + "'")); - boolean simplerAcl = Flags.SIMPLER_ACL.bindTo(nodeRepository.flagSource()).value(); - List<NodeAcl> acls = aclsForChildren ? nodeRepository.getChildAcls(node, simplerAcl) : - List.of(node.acl(nodeRepository.nodes().list(), nodeRepository.loadBalancers(), nodeRepository.zone(), simplerAcl)); + List<NodeAcl> acls = aclsForChildren ? nodeRepository.getChildAcls(node) : + List.of(node.acl(nodeRepository.nodes().list(), nodeRepository.loadBalancers(), nodeRepository.zone())); Cursor trustedNodesArray = object.setArray("trustedNodes"); acls.forEach(nodeAcl -> toSlime(nodeAcl, trustedNodesArray)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java index 26925372b93..94014712930 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java @@ -58,7 +58,7 @@ public class AclProvisioningTest { // Get trusted nodes for the first active node Node node = activeNodes.get(0); List<Node> hostOfNode = node.parentHostname().flatMap(tester.nodeRepository().nodes()::node).map(List::of).orElseGet(List::of); - Supplier<NodeAcl> nodeAcls = () -> node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true); + Supplier<NodeAcl> nodeAcls = () -> node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone()); // Trusted nodes are active nodes in same application, proxy nodes and config servers assertAcls(trustedNodesOf(List.of(activeNodes, proxyNodes, configServers.asList(), hostOfNode), node.cloudAccount()), @@ -83,7 +83,7 @@ public class AclProvisioningTest { // Get trusted nodes for a parked tenant node Node node = tester.nodeRepository().nodes().list(Node.State.parked).nodeType(NodeType.tenant).first().get(); - NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true); + NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone()); // Trusted nodes are all config-nodes assertAcls(trustedNodesOf(List.of(proxyNodes, configServers.asList()), node.cloudAccount()), List.of(nodeAcl)); @@ -108,7 +108,7 @@ public class AclProvisioningTest { // Get trusted nodes for the first config server Node node = tester.nodeRepository().nodes().node("cfg1") .orElseThrow(() -> new RuntimeException("Failed to find cfg1")); - NodeAcl nodeAcl = node.acl(nodes, tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true); + NodeAcl nodeAcl = node.acl(nodes, tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone()); // Trusted nodes is all tenant nodes, all proxy nodes, all config servers and load balancer subnets // All tenant hosts because nodes are IPv6 and cfg are IPv4, so traffic is NATed. @@ -128,7 +128,7 @@ public class AclProvisioningTest { publicTester.makeConfigServers(3, "default", Version.fromString("6.123.456")); Node publicCfgNode = publicTester.nodeRepository().nodes().node("cfg1") .orElseThrow(() -> new RuntimeException("Failed to find cfg1")); - NodeAcl publicNodeAcl = publicCfgNode.acl(nodes, publicTester.nodeRepository().loadBalancers(), publicTester.nodeRepository().zone(), true); + NodeAcl publicNodeAcl = publicCfgNode.acl(nodes, publicTester.nodeRepository().loadBalancers(), publicTester.nodeRepository().zone()); assertEquals(Set.of(51820), publicNodeAcl.trustedUdpPorts()); } @@ -146,7 +146,7 @@ public class AclProvisioningTest { // Get trusted nodes for first proxy node NodeList proxyNodes = tester.nodeRepository().nodes().list().nodeType(NodeType.proxy); Node node = proxyNodes.first().get(); - NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true); + NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone()); // Trusted nodes is all config servers and all proxy nodes assertAcls(trustedNodesOf(List.of(proxyNodes.asList(), configServers.asList()), node.cloudAccount()), List.of(nodeAcl)); @@ -164,7 +164,7 @@ public class AclProvisioningTest { List<Node> nodes = tester.makeReadyChildren(5, new NodeResources(1, 4, 10, 1), host.hostname()); - List<NodeAcl> acls = tester.nodeRepository().getChildAcls(host, true); + List<NodeAcl> acls = tester.nodeRepository().getChildAcls(host); // ACLs for each container on the host assertFalse(nodes.isEmpty()); @@ -188,7 +188,7 @@ public class AclProvisioningTest { List<Node> controllers = tester.nodeRepository().nodes().list().nodeType(NodeType.controller).asList(); // Controllers and hosts all trust each other - NodeAcl controllerAcl = controllers.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true); + NodeAcl controllerAcl = controllers.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone()); assertAcls(trustedNodesOf(List.of(controllers), controllers.get(0).cloudAccount()), Set.of("10.2.3.0/24", "10.4.5.0/24"), List.of(controllerAcl)); assertEquals(Set.of(22, 4443, 443), controllerAcl.trustedPorts()); assertEquals(Set.of(), controllerAcl.trustedUdpPorts()); @@ -217,7 +217,7 @@ public class AclProvisioningTest { // ACL for nodes with allocation trust their respective load balancer networks, if any for (var host : hosts) { - List<NodeAcl> acls = tester.nodeRepository().getChildAcls(host, true); + List<NodeAcl> acls = tester.nodeRepository().getChildAcls(host); assertEquals(2, acls.size()); for (var acl : acls) { if (acl.node().allocation().isPresent()) { @@ -235,7 +235,7 @@ public class AclProvisioningTest { tester.makeConfigServers(3, "default", Version.fromString("6.123.456")); List<Node> readyNodes = tester.makeReadyNodes(1, "default", NodeType.proxy); - NodeAcl nodeAcl = readyNodes.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true); + NodeAcl nodeAcl = readyNodes.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone()); assertEquals(3, nodeAcl.trustedNodes().size()); assertEquals(List.of(Set.of("127.0.1.1"), Set.of("127.0.1.2"), Set.of("127.0.1.3")), |