Closed markuman closed 9 months ago
@markuman I can't reproduce the issue when modifying the existing load balancer rule.
The only issue I can reproduce is when creating a new load balancer rule using both UseExistingClientSecret=true
and ClientSecret
The error was: botocore.errorfactory.InvalidLoadBalancerActionException: An error occurred (InvalidLoadBalancerAction) when calling the CreateRule operation: You cannot both specify a client secret and set UseExistingClientSecret to true
However this has been fixed using #1956
Could you please provide a full sequence on how to reproduce the issue ? Here is the playbook I used to reproduce
- name: Create an ALB with different listener by adding rule
amazon.aws.elb_application_lb:
name: sample-lb
subnets:
- subnet-xxxxxxxxxxxxxxxxx
- subnet-xxxxxxxxxxxxxxxxx
security_groups: sg-xxxxxxxxxxxxxx
state: present
listeners:
- Protocol: HTTPS
Port: 443
Certificates:
- CertificateArn: arn:aws:iam::0123456789:server-certificate/ansible-test-xxxxxxxxxxx
SslPolicy: ELBSecurityPolicy-TLS13-1-2-2021-06
DefaultActions:
- Type: forward
TargetGroupName: test-target-01
Rules:
- Priority: 1
Conditions:
- Field: path-pattern
Values:
- /test
Actions:
- TargetGroupName: test-target-01
Type: forward
Order: 2
- Type: authenticate-oidc
Order: 1
AuthenticateOidcConfig:
Issuer: https://xxxxxxxxxxx
AuthorizationEndpoint: https://xxxxxxxxxxxxxx
TokenEndpoint: https://xxxxxxxxxxxxxx/oauth/token
UserInfoEndpoint: https://xxxxxxxxxxxxx/userinfo
ClientId: myclientid123645
ClientSecret: abcdefghigjth1233
UseExistingClientSecret: True
The modified rule is correct returned at line 1186: https://github.com/ansible-collections/amazon.aws/blob/main/plugins/module_utils/elbv2.py#L1186
The key ClientSecret
is popped, because UseExistingClientSecret
is set to True
.
To make it visible, I add some q
debugging lines.
diff --git a/plugins/module_utils/elbv2.py b/plugins/module_utils/elbv2.py
index 429d5f7c..e69615f1 100644
--- a/plugins/module_utils/elbv2.py
+++ b/plugins/module_utils/elbv2.py
@@ -95,8 +95,12 @@ def _prune_ForwardConfig(action):
# remove the client secret if UseExistingClientSecret, because aws won't return it
# add default values when they are not requested
def _prune_secret(action):
+ import q
+
if action["Type"] != "authenticate-oidc":
return action
+ else:
+ q(action)
if not action["AuthenticateOidcConfig"].get("Scope", False):
action["AuthenticateOidcConfig"]["Scope"] = "openid"
@@ -105,7 +109,9 @@ def _prune_secret(action):
action["AuthenticateOidcConfig"]["SessionTimeout"] = 604800
if action["AuthenticateOidcConfig"].get("UseExistingClientSecret", False):
+ q("must be popped")
action["AuthenticateOidcConfig"].pop("ClientSecret", None)
+ q(action)
return action
@@ -1131,7 +1137,7 @@ class ELBListenerRules:
:return:
"""
-
+ import q
rules_to_modify = []
rules_to_delete = []
rules_to_add = deepcopy(self.rules)
@@ -1147,6 +1153,7 @@ class ELBListenerRules:
if modified_rule:
modified_rule["Priority"] = int(current_rule["Priority"])
modified_rule["RuleArn"] = current_rule["RuleArn"]
+ q("prune secret will be reverted here")
modified_rule["Actions"] = new_rule["Actions"]
modified_rule["Conditions"] = new_rule["Conditions"]
rules_to_modify.append(modified_rule)
@@ -1155,7 +1162,7 @@ class ELBListenerRules:
# 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"]:
rules_to_delete.append(current_rule["RuleArn"])
-
+ q(rules_to_modify)
return rules_to_add, rules_to_modify, rules_to_delete
And after that, in Line 1190, the modified rule, where the ClientSecret
was popped (because it is necessary), it is reverted by taken the requested rule again. So the popped ClientSecret
is in place again and both - ClientSecret
And UseExistingClientSecret: True
is requested against boto3.
4.2s _prune_secret:
action={'AuthenticateOidcConfig': {'AuthorizationEndpoint': 'https://login.microsoftonline.com/123/oauth2/v2.0/authorize', 'ClientId': 'ABClientID', 'ClientSecret': 'xxx', 'Issuer': 'https://login.microsoftonline.com/123/v2.0', 'OnUnauthenticatedRequest': 'authenticate', 'SessionCookieName': 'AWSELBAuthSessionCookie', 'TokenEndpoint': 'https://login.microsoftonline.com/123/oauth2/v2.0/token', 'UseExistingClientSecret': True, 'UserInfoEndpoint': 'https://graph.microsoft.com/oidc/userinfo'}, 'Order': 1, 'Type': 'authenticate-oidc'}
4.2s _prune_secret: "must be popped"='must be popped'
4.2s _prune_secret:
action={'AuthenticateOidcConfig': {'AuthorizationEndpoint': 'https://login.microsoftonline.com/123/oauth2/v2.0/authorize', 'ClientId': 'ABClientID', 'Issuer': 'https://login.microsoftonline.com/123/v2.0', 'OnUnauthenticatedRequest': 'authenticate', 'Scope': 'openid', 'SessionCookieName': 'AWSELBAuthSessionCookie', 'SessionTimeout': 604800, 'TokenEndpoint': 'https://login.microsoftonline.com/123/oauth2/v2.0/token', 'UseExistingClientSecret': True, 'UserInfoEndpoint': 'https://graph.microsoft.com/oidc/userinfo'}, 'Order': 1, 'Type': 'authenticate-oidc'}
4.2s compare_rules:
"prune secret will be reverted here"='prune secret will be reverted here'
4.2s compare_rules:
"prune secret will be reverted here"='prune secret will be reverted here'
4.2s compare_rules:
rules_to_modify=[{'Actions': [{'AuthenticateOidcConfig': {'AuthorizationEndpoint': 'https://login.microsoftonline.com/123/oauth2/v2.0/authorize', 'ClientId': 'ABClientID', 'ClientSecret': 'xxx', 'Issuer': 'https://login.microsoftonline.com/123/v2.0', 'OnUnauthenticatedRequest': 'authenticate', 'SessionCookieName': 'AWSELBAuthSessionCookie', 'TokenEndpoint': 'https://login.microsoftonline.com/123/oauth2/v2.0/token', 'UseExistingClientSecret': True, 'UserInfoEndpoint': 'https://graph.microsoft.com/oidc/userinfo'}, 'Order': 1, 'Type': 'authenticate-oidc'}, {'Order': 2, 'TargetGroupArn': 'arn:aws:elasticloadbalancing:eu-central-1:111:targetgroup/extern/c68839a5495', 'Type': 'forward'}], 'Conditions': [{'Field': 'host-header', 'Values': ['something1.lekker.de', 'something.lekker.de']}, {'Field': 'path-pattern', 'Values': ['/be.php/*']}], 'Priority': 25, 'RuleArn': 'arn:aws:elasticloadbalancing:eu-central-1:111:listener-rule/app/lekker/fba205cf17b1/c2a8d42098/7922f23be698'}, {'Actions': [{'TargetGroupArn': 'arn:aws:elasticloadbalancing:eu-central-1:111:targetgroup/ecs-energieladen-extern/dd40d9226b35', 'Type': 'forward'}], 'Conditions': [{'Field': 'path-pattern', 'Values': ['/wp-*']}], 'Priority': 47, 'RuleArn': 'arn:aws:elasticloadbalancing:eu-central-1:111:listener-rule/app/lekker/fba205cf17b9a51/c2a8420d798/eee95bdede5f529b'}]
Okey, the issue is a kind different.
The rule was detected as a modified rule, but it was a new one.
Why? Because it was added somewhere in the middle fo the rules list and ansible just used the priority
key to say if a rule is a modified or a new rule https://github.com/ansible-collections/amazon.aws/blob/main/plugins/module_utils/elbv2.py#L1182
This case is not solveable!
But when the rule is added at the end of the rules list, it becomes a new rule (priority that does not exist yet), the module runs into a hen-egg problem.
You want to commit the rule with UseExistingClientSecret: True
to become immutable for multiple runs of your playbook, you'll run into the same error
"msg": "An error occurred (InvalidLoadBalancerAction) when calling the ModifyRule operation: You cannot both specify a client secret and set UseExistingClientSecret to true
When
authenticate-oidc
ANDrules_to_add
)then UseExistingClientSecret
must be set to False
, no matter what was requested.
So to reproduce it you need to first apply this
- name: Create an ALB with different listener by adding rule
amazon.aws.elb_application_lb:
name: sample-lb
subnets:
- subnet-xxxxxxxxxxxxxxxxx
- subnet-xxxxxxxxxxxxxxxxx
security_groups: sg-xxxxxxxxxxxxxx
state: present
listeners:
- Protocol: HTTPS
Port: 443
Certificates:
- CertificateArn: arn:aws:iam::0123456789:server-certificate/ansible-test-xxxxxxxxxxx
SslPolicy: ELBSecurityPolicy-TLS13-1-2-2021-06
DefaultActions:
- Type: forward
TargetGroupName: test-target-01
Rules:
- Priority: 1
Conditions:
- Field: host-header
Values:
- bla.tld
Actions:
- TargetGroupName: somewhere
Type: forward
- Priority: 2
Conditions:
- Field: host-header
Values:
- yolo.rocks
Actions:
- TargetGroupName: yeah
Type: forward
and then modify it like that
- name: Create an ALB with different listener by adding rule
amazon.aws.elb_application_lb:
name: sample-lb
subnets:
- subnet-xxxxxxxxxxxxxxxxx
- subnet-xxxxxxxxxxxxxxxxx
security_groups: sg-xxxxxxxxxxxxxx
state: present
listeners:
- Protocol: HTTPS
Port: 443
Certificates:
- CertificateArn: arn:aws:iam::0123456789:server-certificate/ansible-test-xxxxxxxxxxx
SslPolicy: ELBSecurityPolicy-TLS13-1-2-2021-06
DefaultActions:
- Type: forward
TargetGroupName: test-target-01
Rules:
- Priority: 1
Conditions:
- Field: host-header
Values:
- bla.tld
Actions:
- TargetGroupName: somewhere
Type: forward
- Priority: 2
Conditions:
- Field: path-pattern
Values:
- /test
Actions:
- TargetGroupName: test-target-01
Type: forward
Order: 2
- Type: authenticate-oidc
Order: 1
AuthenticateOidcConfig:
Issuer: https://xxxxxxxxxxx
AuthorizationEndpoint: https://xxxxxxxxxxxxxx
TokenEndpoint: https://xxxxxxxxxxxxxx/oauth/token
UserInfoEndpoint: https://xxxxxxxxxxxxx/userinfo
ClientId: myclientid123645
ClientSecret: abcdefghigjth1233
UseExistingClientSecret: True
- Priority: 3
Conditions:
- Field: host-header
Values:
- yolo.rocks
Actions:
- TargetGroupName: yeah
Type: forward
@markuman I have updated PR #1956 to fix this test case. Could you please validate if it is working as expected using #1956? Thanks
@markuman I have updated PR #1956 to fix this test case. Could you please validate if it is working as expected using #1956? Thanks
No, it does not solve the issue.
When you add an new authenticate-oidc
rule in the middle of your existing rules, .... the rule is still detected as a changed one (because comparison is based on priority number), but strictly speaking, it is a new one.
When
1. Rule is from action type `authenticate-oidc` **AND** 2. Rule is a new one (`rules_to_add`)
then
UseExistingClientSecret
must be set toFalse
, no matter what was requested.
this still failed with your branch, when adding a new rule to existing ALB at the end (priority number + 1).
@markuman I just realized that there is an API to update the Rule priority, this will be used when the rule has just changed the priority but all the other properties remain the same. This will fix the use case where an authenticate-oidc
rule is inserted in the middle of existing rules.
When
1. Rule is from action type `authenticate-oidc` **AND** 2. Rule is a new one (`rules_to_add`)
then
UseExistingClientSecret
must be set toFalse
, no matter what was requested.this still failed with your branch, when adding a new rule to existing ALB at the end (priority number + 1).
This has also been fixed.
So to reproduce it you need to first apply this
- name: Create an ALB with different listener by adding rule amazon.aws.elb_application_lb: name: sample-lb subnets: - subnet-xxxxxxxxxxxxxxxxx - subnet-xxxxxxxxxxxxxxxxx security_groups: sg-xxxxxxxxxxxxxx state: present listeners: - Protocol: HTTPS Port: 443 Certificates: - CertificateArn: arn:aws:iam::0123456789:server-certificate/ansible-test-xxxxxxxxxxx SslPolicy: ELBSecurityPolicy-TLS13-1-2-2021-06 DefaultActions: - Type: forward TargetGroupName: test-target-01 Rules: - Priority: 1 Conditions: - Field: host-header Values: - bla.tld Actions: - TargetGroupName: somewhere Type: forward - Priority: 2 Conditions: - Field: host-header Values: - yolo.rocks Actions: - TargetGroupName: yeah Type: forward
and then modify it like that
- name: Create an ALB with different listener by adding rule amazon.aws.elb_application_lb: name: sample-lb subnets: - subnet-xxxxxxxxxxxxxxxxx - subnet-xxxxxxxxxxxxxxxxx security_groups: sg-xxxxxxxxxxxxxx state: present listeners: - Protocol: HTTPS Port: 443 Certificates: - CertificateArn: arn:aws:iam::0123456789:server-certificate/ansible-test-xxxxxxxxxxx SslPolicy: ELBSecurityPolicy-TLS13-1-2-2021-06 DefaultActions: - Type: forward TargetGroupName: test-target-01 Rules: - Priority: 1 Conditions: - Field: host-header Values: - bla.tld Actions: - TargetGroupName: somewhere Type: forward - Priority: 2 Conditions: - Field: path-pattern Values: - /test Actions: - TargetGroupName: test-target-01 Type: forward Order: 2 - Type: authenticate-oidc Order: 1 AuthenticateOidcConfig: Issuer: https://xxxxxxxxxxx AuthorizationEndpoint: https://xxxxxxxxxxxxxx TokenEndpoint: https://xxxxxxxxxxxxxx/oauth/token UserInfoEndpoint: https://xxxxxxxxxxxxx/userinfo ClientId: myclientid123645 ClientSecret: abcdefghigjth1233 UseExistingClientSecret: True - Priority: 3 Conditions: - Field: host-header Values: - yolo.rocks Actions: - TargetGroupName: yeah Type: forward
This issue need to be reopened @abikouo, because it still failed with 7.5.0 the other way round.
Say you've got one ALB with 3 rules. When you delete the one in the middle (priority 2), the module failed
--- /tmp/before.yml 2024-05-03 08:29:41.697584862 +0200
+++ /tmp/after.yml 2024-05-03 08:29:57.211759937 +0200
@@ -24,25 +24,6 @@
Actions:
- TargetGroupName: somewhere
Type: forward
- - Priority: 2
- Conditions:
- - Field: path-pattern
- Values:
- - /test
- Actions:
- - TargetGroupName: test-target-01
- Type: forward
- Order: 2
- - Type: authenticate-oidc
- Order: 1
- AuthenticateOidcConfig:
- Issuer: https://xxxxxxxxxxx
- AuthorizationEndpoint: https://xxxxxxxxxxxxxx
- TokenEndpoint: https://xxxxxxxxxxxxxx/oauth/token
- UserInfoEndpoint: https://xxxxxxxxxxxxx/userinfo
- ClientId: myclientid123645
- ClientSecret: abcdefghigjth1233
- UseExistingClientSecret: True
- Priority: 3
Conditions:
- Field: host-header
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.errorfactory.PriorityInUseException: An error occurred (PriorityInUse) when calling the SetRulePriorities operation: One or more priorities not found
fatal: [localhost]: FAILED! => {"boto3_version": "1.34.34", "botocore_version": "1.34.34", "changed": false, "error": {"code": "PriorityInUse", "message": "One or more priorities not found", "type": "Sender"}, "msg": "An error occurred (PriorityInUse) when calling the SetRulePriorities operation: One or more priorities not found", "response_metadata": {"http_headers": {"connection": "close", "content-length": "293", "content-type": "text/xml", "date": "Fri, 03 May 2024 06:26:39 GMT", "x-amzn-requestid": "134a2c29-0060-4014-a048-6fcedd701876"}, "http_status_code": 400, "request_id": "134a2c29-0060-4014-a048-6fcedd701876", "retry_attempts": 0}}
Summary
In the past, you can set as rule
and it worked. Because of this logic, it doesn't matter if the rule is a new one or an existing one.
Currently you get back the error from the past:
once, fixed in https://github.com/ansible-collections/amazon.aws/pull/1270
When removing the
ClientSecret
keyit comes to a new error
So something new has changed/gets broken ....
Issue Type
Bug Report
Component Name
elb_application_lb
Ansible Version
Collection Versions
AWS SDK versions
Configuration
OS / Environment
Ubuntu 22.04
Steps to Reproduce
see in summary
Expected Results
rules are set, doens't matter if both keys
UseExistingClientSecret: True
andClientSecret
are setActual Results
see summary
Code of Conduct