bug5787: added test class with a first few tests regarding ALB rule inserter

2026-09-24 14:38:45 +00:00 · 2023-05-05 00:59:15 +02:00
parent 2d00697117
commit 457a020d5e
4 changed files with 169 additions and 100 deletions
@@ -7,6 +7,8 @@ Bundle-Vendor: SAP
 Fragment-Host: com.sap.sse.landscape.aws.common
 Import-Package: org.hamcrest;version="2.2.0",
 org.junit;version="4.13.2",
- org.junit.function;version="4.13.2"
+ org.junit.function;version="4.13.2",
+ software.amazon.awssdk.services.elasticloadbalancingv2.model
 Automatic-Module-Name: com.sap.sse.landscape.aws.common.test
 Bundle-RequiredExecutionEnvironment: JavaSE-1.8
+Require-Bundle: com.sap.sse.landscape.aws
@@ -0,0 +1,125 @@
+package com.sap.sse.landscape.aws.common.shared;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import com.sap.sse.common.Util;
+import com.sap.sse.common.Util.Pair;
+import com.sap.sse.landscape.aws.ApplicationLoadBalancer;
+import com.sap.sse.landscape.aws.impl.LoadBalancerRuleInserter;
+import com.sap.sse.landscape.aws.impl.LoadBalancerRuleInserter.LoadBalancerAdapter;
+import com.sap.sse.landscape.aws.impl.LoadBalancerRuleInserter.RuleAdapter;
+
+public class LoadBalancerRuleInserterTest {
+    private LoadBalancerAdapter<TestRuleAdapter> loadBalancerAdapter;
+    private LoadBalancerRuleInserter<String, TestRuleAdapter> ruleInserter;
+
+    private static class TestRuleAdapter implements RuleAdapter<TestRuleAdapter> {
+        private final boolean isDefault;
+        private final int priority;
+        private final String ruleArn;
+        
+        public TestRuleAdapter(boolean isDefault, int priority, String ruleArn) {
+            super();
+            this.isDefault = isDefault;
+            this.priority = priority;
+            this.ruleArn = ruleArn;
+        }
+
+        @Override
+        public boolean isDefault() {
+            return isDefault;
+        }
+
+        @Override
+        public String priority() {
+            return ""+priority;
+        }
+
+        @Override
+        public String ruleArn() {
+            return ruleArn;
+        }
+
+        @Override
+        public TestRuleAdapter copyWithNewPriority(int priorityToUseForRuleCopy) {
+            return new TestRuleAdapter(isDefault, priorityToUseForRuleCopy, ruleArn);
+        }
+    }
+
+    private static class TestLoadBalancerAdapter implements LoadBalancerAdapter<TestRuleAdapter> {
+        private static final long serialVersionUID = -4853040303405813921L;
+        private Iterable<TestRuleAdapter> rules;
+
+        @Override
+        public String getName() {
+            return "Test Load Balancer Adapter";
+        }
+
+        @Override
+        public Iterable<TestRuleAdapter> getRules() {
+            return rules;
+        }
+
+        @Override
+        public void updateLoadBalancerListenerRulePriorities(
+                List<Pair<Integer, TestRuleAdapter>> newPrioritiesForExistingRules) {
+            rules = Util.map(newPrioritiesForExistingRules, p -> p.getB().copyWithNewPriority(p.getA()));
+        }
+
+        @Override
+        public void addRules(List<TestRuleAdapter> rulesToAdd) {
+            rules = Util.concat(Arrays.asList(rules, rulesToAdd));
+        }
+    }
+
+    @Before
+    public void setUp() {
+        loadBalancerAdapter = new TestLoadBalancerAdapter();
+        loadBalancerAdapter.addRules(Arrays.asList(new TestRuleAdapter(/* isDefault */ true, 0, "default")));
+        ruleInserter = new LoadBalancerRuleInserter<>(
+                loadBalancerAdapter, ApplicationLoadBalancer.MAX_PRIORITY, ApplicationLoadBalancer.MAX_RULES_PER_LOADBALANCER);
+        assertUniqueAscendingPriorities();
+    }
+    
+    @Test
+    public void simpleRuleInsertionTest() {
+        assertEquals(1, Util.size(loadBalancerAdapter.getRules()));
+        ruleInserter.addRulesAssigningUnusedPriorities(/* forceContiguous */ true, /* insertBefore */ Optional.empty(),
+                Arrays.asList(new TestRuleAdapter(/* isDefault */ false, 1, "Rule 1")));
+        assertEquals(2, Util.size(loadBalancerAdapter.getRules()));
+        assertUniqueAscendingPriorities();
+    }
+    
+    @Test
+    public void additionalRuleInsertionTest() {
+        assertEquals(1, Util.size(loadBalancerAdapter.getRules()));
+        ruleInserter.addRulesAssigningUnusedPriorities(/* forceContiguous */ true, /* insertBefore */ Optional.empty(),
+                Arrays.asList(new TestRuleAdapter(/* isDefault */ false, 1, "Rule 1")));
+        assertEquals(2, Util.size(loadBalancerAdapter.getRules()));
+        ruleInserter.addRulesAssigningUnusedPriorities(/* forceContiguous */ true, /* insertBefore */ Optional.empty(),
+                Arrays.asList(new TestRuleAdapter(/* isDefault */ false, 1, "Rule 2")));
+        assertUniqueAscendingPriorities();
+    }
+    
+    // TODO add tests where "holes" in the rules base do / don't allow for a sequence of rules to be added contiguously
+    
+    // TODO add tests regarding inserting at another Rule's position, shifting other rules "right"
+    
+    // TODO assert that exceptions are thrown if there is not enough space
+    
+    private void assertUniqueAscendingPriorities() {
+        int lastPriority = -1;
+        for (final TestRuleAdapter rule : loadBalancerAdapter.getRules()) {
+            assertTrue(Integer.valueOf(rule.priority()) > lastPriority);
+            lastPriority = Integer.valueOf(rule.priority());
+        }
+    }
+}
@@ -1,11 +1,10 @@
 package com.sap.sse.landscape.aws.impl;

 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map.Entry;
 import java.util.Optional;
@@ -21,6 +20,7 @@ import com.sap.sse.landscape.aws.ApplicationLoadBalancer;
 import com.sap.sse.landscape.aws.AwsLandscape;
 import com.sap.sse.landscape.aws.TargetGroup;
 import com.sap.sse.landscape.aws.common.shared.PlainRedirectDTO;
+import com.sap.sse.landscape.aws.impl.LoadBalancerRuleInserter.ALBRuleAdapter;

 import software.amazon.awssdk.services.elasticloadbalancingv2.model.Action;
 import software.amazon.awssdk.services.elasticloadbalancingv2.model.ActionTypeEnum;
@@ -208,74 +208,10 @@ implements ApplicationLoadBalancer<ShardingKey> {

    @Override
    public Iterable<Rule> addRulesAssigningUnusedPriorities(boolean forceContiguous, Optional<Rule> insertBefore, Rule... rules) {
-        // TODO bug5787: allow caller to specify an insertion point; either as an index or as a rule before which to insert
-        final Iterable<Rule> existingRules = getRules();
-        if (Util.size(existingRules)-1 + rules.length > MAX_PRIORITY) { // -1 due to the default rule being part of existingRules
-            throw new IllegalArgumentException("The "+rules.length+" new rules won't find enough unused priority numbers because there are already "+
-                    (Util.size(existingRules)-1)+" of them and together they would exceed the maximum of "+MAX_PRIORITY+" by "+
-                    (Util.size(existingRules)-1 + rules.length - MAX_PRIORITY));
-        }
-        if (Util.size(existingRules) + rules.length > MAX_RULES_PER_LOADBALANCER) {
-            throw new IllegalArgumentException("The " + rules.length + " new rules would make the ALB " + getName()
-                    + " exceed its maximum number of rules (" + MAX_PRIORITY + ") by "
-                    + (Util.size(existingRules) + rules.length - MAX_RULES_PER_LOADBALANCER));
-        }
-        final List<Rule> result = new ArrayList<>(rules.length);
-        final List<Rule> sortedExistingNonDefaultRules = new ArrayList<>(Util.size(existingRules)-1);
-        Util.addAll(Util.filter(existingRules, r->!r.isDefault()), sortedExistingNonDefaultRules);
-        Collections.sort(sortedExistingNonDefaultRules, (r1, r2)->Integer.valueOf(r1.priority()).compareTo(Integer.valueOf(r2.priority())));
-        final int stepwidth;
-        if (forceContiguous) {
-            stepwidth = rules.length;
-        } else {
-            stepwidth = 1;
-        }
-        int rulesIndex = 0;
-        int previousPriority = 0;
-        final Iterator<Rule> existingRulesIter = sortedExistingNonDefaultRules.iterator();
-        while (rulesIndex < rules.length) {
-            // find next available slot
-            int nextPriority = MAX_PRIORITY+1; // if no further rule exists, the usable gap ends after MAX_PRIORITY
-            final Rule[] nextRule = new Rule[1]; // using an array to make it final so we can access it inside the mapping function below
-            boolean stillLookingForRuleToInsertBefore = insertBefore.isPresent();
-            while (existingRulesIter.hasNext() &&
-                    ((nextPriority=Integer.valueOf((nextRule[0]=existingRulesIter.next()).priority())) <= previousPriority+stepwidth) ||
-                     (stillLookingForRuleToInsertBefore && (stillLookingForRuleToInsertBefore=insertBefore.map(rule->!rule.ruleArn().equals(nextRule[0].ruleArn())).orElse(false)))) {
-                // not enough space for stepwidth many rules; keep on searching
-                previousPriority = nextPriority;
-                if (!existingRulesIter.hasNext()) {
-                    nextPriority = MAX_PRIORITY+1;
-                }
-            }
-            if (previousPriority+stepwidth > MAX_PRIORITY) {
-                if (forceContiguous) {
-                    previousPriority = squeezeExistingRulesAndReturnLastUsedPriority(sortedExistingNonDefaultRules);
-                    nextPriority = MAX_PRIORITY+1;
-                    // we previously checked already that there is enough room for the new set of rules
-                    assert previousPriority + rules.length <= MAX_PRIORITY;
-                } else {
-                    throw new IllegalStateException(
-                            "The " + rules.length + " new rules don't fit into the existing rule set of load balancer "
-                                    + getName() + " without exceeding the maximum priority of " + MAX_PRIORITY);
-                }
-            }
-            while (rulesIndex < rules.length && ++previousPriority < nextPriority) {
-                final int priorityToUseForNextRule = previousPriority;
-                result.add(rules[rulesIndex++].copy(b->b.priority(""+priorityToUseForNextRule)));
-            }
-        }
-        addRules(result.toArray(new Rule[0]));
-        return result;
-    }
-
-    private int squeezeExistingRulesAndReturnLastUsedPriority(final List<Rule> sortedExistingNonDefaultRules) {
-        final List<RulePriorityPair> newPrioritiesForExistingRules = new LinkedList<>();
-        int priority = 0;
-        for (final Rule existingRule : sortedExistingNonDefaultRules) {
-            newPrioritiesForExistingRules.add(RulePriorityPair.builder().ruleArn(existingRule.ruleArn()).priority(++priority).build());
-        }
-        landscape.updateLoadBalancerListenerRulePriorities(getRegion(), newPrioritiesForExistingRules);
-        return priority;
+        final List<Rule> rulesAsList = Arrays.asList(rules);
+        return Util.map(LoadBalancerRuleInserter.create(this, MAX_PRIORITY, MAX_RULES_PER_LOADBALANCER).addRulesAssigningUnusedPriorities(
+                forceContiguous, insertBefore.map(LoadBalancerRuleInserter::createRuleAdapter),
+                Util.map(rulesAsList, LoadBalancerRuleInserter::createRuleAdapter)), ALBRuleAdapter::getRule);
    }

    @Override
@@ -29,19 +29,19 @@ import software.amazon.awssdk.services.elasticloadbalancingv2.model.RulePriority
 * @author Axel Uhl (d043530)
 *
 */
-public class LoadBalancerRuleInserter<ShardingKey, RA extends com.sap.sse.landscape.aws.impl.LoadBalancerRuleInserter.RuleAdapter> {
+public class LoadBalancerRuleInserter<ShardingKey, RA extends com.sap.sse.landscape.aws.impl.LoadBalancerRuleInserter.RuleAdapter<RA>> {
    private final LoadBalancerAdapter<RA> loadBalancerAdapter;
    private final int maxPriority;
-    private final int maxRulesParLoadBalancer;
+    private final int maxRulesPerLoadBalancer;
    
-    public static interface RuleAdapter {
+    public static interface RuleAdapter<RA extends RuleAdapter<RA>> {
        boolean isDefault();
        String priority();
        String ruleArn();
-        RuleAdapter copyWithNewPriority(int priorityToUseForRuleCopy);
+        RA copyWithNewPriority(int priorityToUseForRuleCopy);
    }
    
-    public static interface LoadBalancerAdapter<RA extends RuleAdapter> extends Named {
+    public static interface LoadBalancerAdapter<RA extends RuleAdapter<RA>> extends Named {
        Iterable<RA> getRules();

        void updateLoadBalancerListenerRulePriorities(List<Pair<Integer, RA>> newPrioritiesForExistingRules);
@@ -49,7 +49,7 @@ public class LoadBalancerRuleInserter<ShardingKey, RA extends com.sap.sse.landsc
        void addRules(List<RA> result);
    }
    
-    private static class ALBRuleAdapter implements RuleAdapter {
+    public static class ALBRuleAdapter implements RuleAdapter<ALBRuleAdapter> {
        private final Rule rule;
        
        private ALBRuleAdapter(Rule rule) {
@@ -72,7 +72,7 @@ public class LoadBalancerRuleInserter<ShardingKey, RA extends com.sap.sse.landsc
        }

        @Override
-        public RuleAdapter copyWithNewPriority(int priorityToUseForRuleCopy) {
+        public ALBRuleAdapter copyWithNewPriority(int priorityToUseForRuleCopy) {
            return new ALBRuleAdapter(rule.copy(b->b.priority(""+priorityToUseForRuleCopy).build()));
        }
        
@@ -114,14 +114,19 @@ public class LoadBalancerRuleInserter<ShardingKey, RA extends com.sap.sse.landsc
        
    }
    
-    public LoadBalancerRuleInserter(LoadBalancerAdapter<RA> loadBalancerAdapter, int maxPriority, int maxRulesParLoadBalancer) {
+    public LoadBalancerRuleInserter(LoadBalancerAdapter<RA> loadBalancerAdapter, int maxPriority, int maxRulesPerLoadBalancer) {
        super();
        this.loadBalancerAdapter = loadBalancerAdapter;
        this.maxPriority = maxPriority;
-        this.maxRulesParLoadBalancer = maxRulesParLoadBalancer;
+        this.maxRulesPerLoadBalancer = maxRulesPerLoadBalancer;
    }
    
-    static ALBRuleAdapter createRuleAdapter(Rule rule) {
+    public static <ShardingKey> LoadBalancerRuleInserter<ShardingKey, ALBRuleAdapter> create(
+            ApplicationLoadBalancer<ShardingKey> alb, int maxPriority, int maxRulesPerLoadBalancer) {
+        return new LoadBalancerRuleInserter<>(createLoadBalancerAdapter(alb), maxPriority, maxRulesPerLoadBalancer);
+    }
+    
+    public static ALBRuleAdapter createRuleAdapter(Rule rule) {
        return new ALBRuleAdapter(rule);
    }
    
@@ -129,35 +134,37 @@ public class LoadBalancerRuleInserter<ShardingKey, RA extends com.sap.sse.landsc
        return new ALBAdapter<ShardingKey>(alb);
    }

-    public List<RuleAdapter> addRulesAssigningUnusedPriorities(boolean forceContiguous, Optional<RA> insertBefore, RA... rules) {
+    public List<RA> addRulesAssigningUnusedPriorities(boolean forceContiguous, Optional<RA> insertBefore,
+            Iterable<RA> rules) {
        final Iterable<RA> existingRules = loadBalancerAdapter.getRules();
-        if (Util.size(existingRules)-1 + rules.length > maxPriority) { // -1 due to the default rule being part of existingRules
-            throw new IllegalArgumentException("The "+rules.length+" new rules won't find enough unused priority numbers because there are already "+
+        final int rulesSize = Util.size(rules);
+        if (Util.size(existingRules)-1 + rulesSize > maxPriority) { // -1 due to the default rule being part of existingRules
+            throw new IllegalArgumentException("The "+rulesSize+" new rules won't find enough unused priority numbers because there are already "+
                    (Util.size(existingRules)-1)+" of them and together they would exceed the maximum of "+maxPriority+" by "+
-                    (Util.size(existingRules)-1 + rules.length - maxPriority));
+                    (Util.size(existingRules)-1 + rulesSize - maxPriority));
        }
-        if (Util.size(existingRules) + rules.length > maxRulesParLoadBalancer) {
-            throw new IllegalArgumentException("The " + rules.length + " new rules would make the ALB " + loadBalancerAdapter.getName()
+        if (Util.size(existingRules) + rulesSize > maxRulesPerLoadBalancer) {
+            throw new IllegalArgumentException("The " + rulesSize + " new rules would make the ALB " + loadBalancerAdapter.getName()
                    + " exceed its maximum number of rules (" + maxPriority + ") by "
-                    + (Util.size(existingRules) + rules.length - maxRulesParLoadBalancer));
+                    + (Util.size(existingRules) + rulesSize - maxRulesPerLoadBalancer));
        }
-        final List<RA> result = new ArrayList<>(rules.length);
+        final List<RA> result = new ArrayList<>(rulesSize);
        final List<RA> sortedExistingNonDefaultRules = new ArrayList<>(Util.size(existingRules)-1);
        Util.addAll(Util.filter(existingRules, r->!r.isDefault()), sortedExistingNonDefaultRules);
        Collections.sort(sortedExistingNonDefaultRules, (r1, r2)->Integer.valueOf(r1.priority()).compareTo(Integer.valueOf(r2.priority())));
        final int stepwidth;
        if (forceContiguous) {
-            stepwidth = rules.length;
+            stepwidth = rulesSize;
        } else {
            stepwidth = 1;
        }
-        int rulesIndex = 0;
+        final Iterator<RA> rulesIterator = rules.iterator();
        int previousPriority = 0;
        final Iterator<RA> existingRulesIter = sortedExistingNonDefaultRules.iterator();
-        while (rulesIndex < rules.length) {
+        while (rulesIterator.hasNext()) {
            // find next available slot
            int nextPriority = maxPriority+1; // if no further rule exists, the usable gap ends after MAX_PRIORITY
-            final RA[] nextRule = new RA[1]; // using an array to make it final so we can access it inside the mapping function below
+            final RuleAdapter<?>[] nextRule = new RuleAdapter<?>[1]; // using an array to make it final so we can access it inside the mapping function below
            boolean stillLookingForRuleToInsertBefore = insertBefore.isPresent();
            while (existingRulesIter.hasNext() &&
                    ((nextPriority=Integer.valueOf((nextRule[0]=existingRulesIter.next()).priority())) <= previousPriority+stepwidth) ||
@@ -173,16 +180,16 @@ public class LoadBalancerRuleInserter<ShardingKey, RA extends com.sap.sse.landsc
                    previousPriority = squeezeExistingRulesAndReturnLastUsedPriority(sortedExistingNonDefaultRules);
                    nextPriority = maxPriority+1;
                    // we previously checked already that there is enough room for the new set of rules
-                    assert previousPriority + rules.length <= maxPriority;
+                    assert previousPriority + rulesSize <= maxPriority;
                } else {
                    throw new IllegalStateException(
-                            "The " + rules.length + " new rules don't fit into the existing rule set of load balancer "
+                            "The " + rulesSize + " new rules don't fit into the existing rule set of load balancer "
                                    + loadBalancerAdapter.getName() + " without exceeding the maximum priority of " + maxPriority);
                }
            }
-            while (rulesIndex < rules.length && ++previousPriority < nextPriority) {
+            while (rulesIterator.hasNext() && ++previousPriority < nextPriority) {
                final int priorityToUseForNextRule = previousPriority;
-                result.add(rules[rulesIndex++].copyWithNewPriority(priorityToUseForNextRule));
+                result.add(rulesIterator.next().copyWithNewPriority(priorityToUseForNextRule));
            }
        }
        loadBalancerAdapter.addRules(result);
@@ -190,13 +197,12 @@ public class LoadBalancerRuleInserter<ShardingKey, RA extends com.sap.sse.landsc
    }
    
    private int squeezeExistingRulesAndReturnLastUsedPriority(final List<RA> sortedExistingNonDefaultRules) {
-        final List<Pair<Integer, RuleAdapter>> newPrioritiesForExistingRules = new LinkedList<>();
+        final List<Pair<Integer, RA>> newPrioritiesForExistingRules = new LinkedList<>();
        int priority = 0;
-        for (final RuleAdapter existingRule : sortedExistingNonDefaultRules) {
+        for (final RA existingRule : sortedExistingNonDefaultRules) {
            newPrioritiesForExistingRules.add(new Pair<>(++priority, existingRule));
        }
        loadBalancerAdapter.updateLoadBalancerListenerRulePriorities(newPrioritiesForExistingRules);
        return priority;
    }
-
 }