module_utils/elbv2 - fix issue with authenticate-oidc listener rule (#…

…1956)

module_utils/elbv2 - fix issue with authenticate-oidc listener rule

SUMMARY

fixes #1877
The module now detect rule which changing priority, they are no more considered as new rules but we are using the set_rule_priorities API instead to update the priority.
For authenticated-oidc rule, we set always set the UseExistingSecret to False for new rule to create and when the rule need to be modified and the user has provided a ClientSecret.

ISSUE TYPE


Bugfix Pull Request

Reviewed-by: Helen Bailey <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Alina Buzachis

changelogs/fragments/20240124-module_utils-elbv2-fix-issue-with-authenticated-oidc.yml

@@ -0,0 +1,6 @@
+---
+bugfixes:
+  - >-
+    module_utils/elbv2 - Fix issue when creating or modifying Load balancer rule
+    type authenticate-oidc using ``ClientSecret`` parameter and
+    ``UseExistingClientSecret=true`` (https://github.com/ansible-collections/amazon.aws/issues/1877).

plugins/module_utils/elbv2.py

@@ -107,6 +107,12 @@ def _prune_secret(action):
     if action["AuthenticateOidcConfig"].get("UseExistingClientSecret", False):
         action["AuthenticateOidcConfig"].pop("ClientSecret", None)
 
+    if not action["AuthenticateOidcConfig"].get("OnUnauthenticatedRequest", False):
+        action["AuthenticateOidcConfig"]["OnUnauthenticatedRequest"] = "authenticate"
+
+    if not action["AuthenticateOidcConfig"].get("SessionCookieName", False):
+        action["AuthenticateOidcConfig"]["SessionCookieName"] = "AWSELBAuthSessionCookie"
+
     return action
 
 
@@ -1010,10 +1016,7 @@ def __init__(self, connection, module, elb_arn, listener_rules, listener_port):
 
         # Get listener based on port so we can use ARN
         self.current_listener = get_elb_listener(connection, module, elb_arn, listener_port)
-        self.listener_arn = self.current_listener["ListenerArn"]
-        self.rules_to_add = deepcopy(self.rules)
-        self.rules_to_modify = []
-        self.rules_to_delete = []
+        self.listener_arn = self.current_listener.get("ListenerArn")
 
         # If the listener exists (i.e. has an ARN) get rules for the listener
         if "ListenerArn" in self.current_listener:
@@ -1141,8 +1144,9 @@ def _compare_rule(self, current_rule, new_rule):
         if len(current_rule["Actions"]) == len(new_rule["Actions"]):
             # if actions have just one element, compare the contents and then update if
             # they're different
+            copy_new_rule = deepcopy(new_rule)
             current_actions_sorted = _sort_actions(current_rule["Actions"])
-            new_actions_sorted = _sort_actions(new_rule["Actions"])
+            new_actions_sorted = _sort_actions(copy_new_rule["Actions"])
 
             new_current_actions_sorted = [_append_use_existing_client_secretn(i) for i in current_actions_sorted]
             new_actions_sorted_no_secret = [_prune_secret(i) for i in new_actions_sorted]
@@ -1175,10 +1179,41 @@ def compare_rules(self):
         rules_to_modify = []
         rules_to_delete = []
         rules_to_add = deepcopy(self.rules)
+        rules_to_set_priority = []
+
+        # List rules to update priority, 'Actions' and 'Conditions' remain the same
+        # only the 'Priority' has changed
+        current_rules = deepcopy(self.current_rules)
+        remaining_rules = []
+        while current_rules:
+            current_rule = current_rules.pop(0)
+            # Skip the default rule, this one can't be modified
+            if current_rule.get("IsDefault", False):
+                continue
+            to_keep = True
+            for new_rule in rules_to_add:
+                modified_rule = self._compare_rule(current_rule, new_rule)
+                if not modified_rule:
+                    # The current rule has been passed with the same properties to the module
+                    # Remove it for later comparison
+                    rules_to_add.remove(new_rule)
+                    to_keep = False
+                    break
+                if modified_rule and list(modified_rule.keys()) == ["Priority"]:
+                    # if only the Priority has changed
+                    modified_rule["Priority"] = int(new_rule["Priority"])
+                    modified_rule["RuleArn"] = current_rule["RuleArn"]
+
+                    rules_to_set_priority.append(modified_rule)
+                    to_keep = False
+                    rules_to_add.remove(new_rule)
+                    break
+            if to_keep:
+                remaining_rules.append(current_rule)
 
-        for current_rule in self.current_rules:
+        for current_rule in remaining_rules:
             current_rule_passed_to_module = False
-            for new_rule in self.rules[:]:
+            for new_rule in rules_to_add:
                 if current_rule["Priority"] == str(new_rule["Priority"]):
                     current_rule_passed_to_module = True
                     # Remove what we match so that what is left can be marked as 'to be added'
@@ -1189,14 +1224,24 @@ def compare_rules(self):
                         modified_rule["RuleArn"] = current_rule["RuleArn"]
                         modified_rule["Actions"] = new_rule["Actions"]
                         modified_rule["Conditions"] = new_rule["Conditions"]
+                        # You cannot both specify a client secret and set UseExistingClientSecret to true
+                        for action in modified_rule.get("Actions", []):
+                            if action.get("AuthenticateOidcConfig", {}).get("ClientSecret", False):
+                                action["AuthenticateOidcConfig"]["UseExistingClientSecret"] = False
                         rules_to_modify.append(modified_rule)
                     break
 
             # If the current rule was not matched against passed rules, mark for removal
-            if not current_rule_passed_to_module and not current_rule["IsDefault"]:
+            if not current_rule_passed_to_module and not current_rule.get("IsDefault", False):
                 rules_to_delete.append(current_rule["RuleArn"])
 
-        return rules_to_add, rules_to_modify, rules_to_delete
+        # For rules to create 'UseExistingClientSecret' should be set to False
+        for rule in rules_to_add:
+            for action in rule.get("Actions", []):
+                if action.get("AuthenticateOidcConfig", {}).get("UseExistingClientSecret", False):
+                    action["AuthenticateOidcConfig"]["UseExistingClientSecret"] = False
+
+        return rules_to_add, rules_to_modify, rules_to_delete, rules_to_set_priority
 
 
 class ELBListenerRule:
@@ -1251,3 +1296,21 @@ def delete(self):
             self.module.fail_json_aws(e)
 
         self.changed = True
+
+    def set_rule_priorities(self):
+        """
+        Sets the priorities of the specified rules.
+
+        :return:
+        """
+
+        try:
+            rules = [self.rule]
+            if isinstance(self.rule, list):
+                rules = self.rule
+            rule_priorities = [{"RuleArn": rule["RuleArn"], "Priority": rule["Priority"]} for rule in rules]
+            AWSRetry.jittered_backoff()(self.connection.set_rule_priorities)(RulePriorities=rule_priorities)
+        except (BotoCoreError, ClientError) as e:
+            self.module.fail_json_aws(e)
+
+        self.changed = True

plugins/modules/elb_application_lb.py

@@ -680,12 +680,22 @@ def create_or_update_alb(alb_obj):
             rules_obj = ELBListenerRules(
                 alb_obj.connection, alb_obj.module, alb_obj.elb["LoadBalancerArn"], listener["Rules"], listener["Port"]
             )
-            rules_to_add, rules_to_modify, rules_to_delete = rules_obj.compare_rules()
+            rules_to_add, rules_to_modify, rules_to_delete, rules_to_set_priority = rules_obj.compare_rules()
 
             # Exit on check_mode
-            if alb_obj.module.check_mode and (rules_to_add or rules_to_modify or rules_to_delete):
+            if alb_obj.module.check_mode and (
+                rules_to_add or rules_to_modify or rules_to_delete or rules_to_set_priority
+            ):
                 alb_obj.module.exit_json(changed=True, msg="Would have updated ALB if not in check mode.")
 
+            # Set rules priorities
+            if rules_to_set_priority:
+                rule_obj = ELBListenerRule(
+                    alb_obj.connection, alb_obj.module, rules_to_set_priority, rules_obj.listener_arn
+                )
+                rule_obj.set_rule_priorities()
+                alb_obj.changed |= rule_obj.changed
+
             # Delete rules
             if alb_obj.module.params["purge_rules"]:
                 for rule in rules_to_delete: