Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

elb_application_lb: rules behavior #1284

Open
1 task done
markuman opened this issue Nov 22, 2022 · 2 comments
Open
1 task done

elb_application_lb: rules behavior #1284

markuman opened this issue Nov 22, 2022 · 2 comments
Labels
bug This issue/PR relates to a bug has_pr python3

Comments

@markuman
Copy link
Member

markuman commented Nov 22, 2022

Summary

The current handling of rules might be resulting always in a changing task.
That depends on action types where boto3 is assuming default values when they are not requested (See https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/elbv2.html and search for The default is).

For example, take action type authenticate-oidc

User request

          - Conditions:
              - Field: host-header
                Values:
                  - some.tld
              - Field: path-pattern
                Values:
                  - "/admin/*"
            Actions:
              - Type: authenticate-oidc
                Order: 1
                AuthenticateOidcConfig:
                  Issuer: https://login.microsoftonline.com/32rw-ewad53te-ef/v2.0
                  AuthorizationEndpoint: https://login.microsoftonline.com/324re-dafs6-6tw/oauth2/v2.0/authorize
                  TokenEndpoint: https://login.microsoftonline.com/432535ez-rfes-32543ter/oauth2/v2.0/token
                  UserInfoEndpoint: https://graph.microsoft.com/oidc/userinfo
                  ClientId: fasgd-235463-fsgd-243
                  ClientSecret: "{{ lookup('onepassword', 'some cool secret', vault='some important vault') }}"
                  SessionCookieName: AWSELBAuthSessionCookie
                  OnUnauthenticatedRequest: authenticate
                  UseExistingClientSecret: True
              - TargetGroupName: "{{ some_tg }}"
                Type: forward
                Order: 2

AWS response for describe rules

          - Conditions:
              - Field: host-header
                Values:
                  - some.tld
              - Field: path-pattern
                Values:
                  - "/admin/*"
            Actions:
              - Type: authenticate-oidc
                Order: 1
                AuthenticateOidcConfig:
                  Issuer: https://login.microsoftonline.com/32rw-ewad53te-ef/v2.0
                  AuthorizationEndpoint: https://login.microsoftonline.com/324re-dafs6-6tw/oauth2/v2.0/authorize
                  TokenEndpoint: https://login.microsoftonline.com/432535ez-rfes-32543ter/oauth2/v2.0/token
                  UserInfoEndpoint: https://graph.microsoft.com/oidc/userinfo
                  ClientId: fasgd-235463-fsgd-243
                  Scope: openid
                  SessionCookieName: AWSELBAuthSessionCookie
                  SessionTimeout: 604800
                  OnUnauthenticatedRequest: authenticate
              - TargetGroupName: "{{ some_tg }}"
                Type: forward
                Order: 2

Module change detect

Note: ClientSecret and UseExistingClientSecret are popped due postprocessing in that case, because AWS won't return none of them, but required one of them.

          - Conditions:
              - Field: host-header
                Values:
                  - some.tld
              - Field: path-pattern
                Values:
                  - "/admin/*"
            Actions:
              - Type: authenticate-oidc
                Order: 1
                AuthenticateOidcConfig:
                  Issuer: https://login.microsoftonline.com/32rw-ewad53te-ef/v2.0
                  AuthorizationEndpoint: https://login.microsoftonline.com/324re-dafs6-6tw/oauth2/v2.0/authorize
                  TokenEndpoint: https://login.microsoftonline.com/432535ez-rfes-32543ter/oauth2/v2.0/token
                  UserInfoEndpoint: https://graph.microsoft.com/oidc/userinfo
                  ClientId: fasgd-235463-fsgd-243
+                Scope: openid
                  SessionCookieName: AWSELBAuthSessionCookie
+                SessionTimeout: 604800
                  OnUnauthenticatedRequest: authenticate
              - TargetGroupName: "{{ some_tg }}"
                Type: forward
                Order: 2
  1. the module will always flag its task as changed when not all defaults value a given
  2. the module will reset values back to default, that are not given by the user.

Point 2. is a very difficult one, because on the one hand, it is a bug. It can result in an unexpected change, e.g. when some properties were set to none-default values via cloudformation, terraform, cdk or even clicked. They got reset to defaults.
But fixing it would also break backwards compatibility :) Because "use the defaults for all values that are not given" is the current behaviour.

I think a control parameter must be introduced, where you can say:

  • a) keep values that AWS returned and are not given by the user
  • b) I don't care about the returned values, just take the defaults (current behaviour)

Issue Type

Bug Report

Component Name

elb_application_alb

Ansible Version

$ ansible --version
ansible --version
/usr/lib/python3/dist-packages/paramiko/transport.py:236: CryptographyDeprecationWarning: Blowfish has been deprecated
  "class": algorithms.Blowfish,
ansible [core 2.13.5]
  config file = None
  configured module search path = ['/home/m/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/m/.local/lib/python3.10/site-packages/ansible
  ansible collection location = /home/m/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/m/.local/bin/ansible
  python version = 3.10.6 (main, Nov  2 2022, 18:53:38) [GCC 11.3.0]
  jinja version = 3.1.2
  libyaml = True

Collection Versions

$ ansible-galaxy collection list

# /home/m/.local/lib/python3.10/site-packages/ansible_collections
Collection                    Version
----------------------------- -------
amazon.aws                    3.5.0  
ansible.netcommon             3.1.3  
ansible.posix                 1.4.0  
ansible.utils                 2.6.1  
ansible.windows               1.11.1 
arista.eos                    5.0.1  
awx.awx                       21.7.0 
azure.azcollection            1.13.0 
check_point.mgmt              2.3.0  
chocolatey.chocolatey         1.3.1  
cisco.aci                     2.2.0  
cisco.asa                     3.1.0  
cisco.dnac                    6.6.0  
cisco.intersight              1.0.19 
cisco.ios                     3.3.2  
cisco.iosxr                   3.3.1  
cisco.ise                     2.5.5  
cisco.meraki                  2.11.0 
cisco.mso                     2.0.0  
cisco.nso                     1.0.3  
cisco.nxos                    3.2.0  
cisco.ucs                     1.8.0  
cloud.common                  2.1.2  
cloudscale_ch.cloud           2.2.2  
community.aws                 3.6.0  
community.azure               1.1.0  
community.ciscosmb            1.0.5  
community.crypto              2.7.0  
community.digitalocean        1.22.0 
community.dns                 2.3.3  
community.docker              2.7.1  
community.fortios             1.0.0  
community.general             5.7.0  
community.google              1.0.0  
community.grafana             1.5.3  
community.hashi_vault         3.3.1  
community.hrobot              1.5.2  
community.libvirt             1.2.0  
community.mongodb             1.4.2  
community.mysql               3.5.1  
community.network             4.0.1  
community.okd                 2.2.0  
community.postgresql          2.2.0  
community.proxysql            1.4.0  
community.rabbitmq            1.2.2  
community.routeros            2.3.0  
community.sap                 1.0.0  
community.sap_libs            1.3.0  
community.skydive             1.0.0  
community.sops                1.4.1  
community.vmware              2.10.0 
community.windows             1.11.0 
community.zabbix              1.8.0  
containers.podman             1.9.4  
cyberark.conjur               1.2.0  
cyberark.pas                  1.0.14 
dellemc.enterprise_sonic      1.1.2  
dellemc.openmanage            5.5.0  
dellemc.os10                  1.1.1  
dellemc.os6                   1.0.7  
dellemc.os9                   1.0.4  
f5networks.f5_modules         1.20.0 
fortinet.fortimanager         2.1.5  
fortinet.fortios              2.1.7  
frr.frr                       2.0.0  
gluster.gluster               1.0.2  
google.cloud                  1.0.2  
hetzner.hcloud                1.8.2  
hpe.nimble                    1.1.4  
ibm.qradar                    2.1.0  
ibm.spectrum_virtualize       1.10.0 
infinidat.infinibox           1.3.3  
infoblox.nios_modules         1.4.0  
inspur.ispim                  1.1.0  
inspur.sm                     2.2.0  
junipernetworks.junos         3.1.0  
kubernetes.core               2.3.2  
mellanox.onyx                 1.0.0  
netapp.aws                    21.7.0 
netapp.azure                  21.10.0
netapp.cloudmanager           21.20.1
netapp.elementsw              21.7.0 
netapp.ontap                  21.24.1
netapp.storagegrid            21.11.1
netapp.um_info                21.8.0 
netapp_eseries.santricity     1.3.1  
netbox.netbox                 3.8.0  
ngine_io.cloudstack           2.2.4  
ngine_io.exoscale             1.0.0  
ngine_io.vultr                1.1.2  
openstack.cloud               1.10.0 
openvswitch.openvswitch       2.1.0  
ovirt.ovirt                   2.2.3  
purestorage.flasharray        1.14.0 
purestorage.flashblade        1.10.0 
purestorage.fusion            1.1.1  
sensu.sensu_go                1.13.1 
servicenow.servicenow         1.0.6  
splunk.es                     2.1.0  
t_systems_mms.icinga_director 1.31.0 
theforeman.foreman            3.7.0  
vmware.vmware_rest            2.2.0  
vultr.cloud                   1.1.0  
vyos.vyos                     3.0.1  
wti.remote                    1.0.4  

# /home/m/.ansible/collections/ansible_collections
Collection         Version   
------------------ ----------
amazon.aws         6.0.0-dev0
ansible.eda        1.3.2     
ansible.posix      1.4.0     
community.aws      5.0.0     
community.crypto   2.3.1     
community.general  5.8.0     
community.mysql    3.2.1     
community.proxysql 1.3.1     
devsec.hardening   8.2.0     
inwx.collection    1.3.0     
lekker.iac         4.0.0     
markuman.nessus    0.0.5     
markuman.nextcloud 9.3.0 

AWS SDK versions

$ pip show boto boto3 botocore

Name: boto3
Version: 1.26.8
Summary: The AWS SDK for Python
Home-page: https://github.com/boto/boto3
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /home/m/.local/lib/python3.10/site-packages
Requires: botocore, jmespath, s3transfer
Required-by: awslogs, deutschland
---
Name: botocore
Version: 1.29.8
Summary: Low-level, data-driven core of boto 3.
Home-page: https://github.com/boto/botocore
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /home/m/.local/lib/python3.10/site-packages
Requires: jmespath, python-dateutil, urllib3
Required-by: boto3, s3transfer

Configuration

$ ansible-config dump --only-changed

OS / Environment

No response

Steps to Reproduce

see summary

Expected Results

see summary

Actual Results

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibullbot
Copy link

Files identified in the description:
None

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug has_pr needs_triage python3 labels Nov 22, 2022
@markuman
Copy link
Member Author

One other possibility rather than introduce a new control parameter might be to documentate any single option for the rules parameter ...

softwarefactory-project-zuul bot pushed a commit that referenced this issue Feb 23, 2023
elbv2: respect UseExistingClientSecret

SUMMARY
Since amazon.aws 5.0.0, elb_application_lb runs into an exception, when using Type: authenticate-oidc in a rule, even when UseExistingClientSecret: True parameter is given. That works as expected with amazon.aws 4.x.x.
The logic gets broken in  #940
Basically AWS won't return both, UseExistingClientSecret and  ClientSecret.
But when requesting against boto3,  both parameters are mutually exclusive!
When the user set UseExistingClientSecret: True, the ClientSecret must be removed for the request.
When the user does not set UseExistingClientSecret or set it to False,  the UseExistingClientSecret must be included in the request.
While diving deeper, I've noticed a basic change detection problem for default values, that are not requested, but AWS will return them. I've summerized it in #1284
However, this PR does not target #1284, it just fixes the exception and restores the functionality and hotfix the change-detection only for Type: authenticate-oidc.
origin PR description

The error was: botocore.errorfactory.InvalidLoadBalancerActionException: An error occurred (InvalidLoadBalancerAction) when calling the ModifyRule operation: You must either specify a client secret or set UseExistingClientSecret to true

UseExistingClientSecret is not respected anymore since a.a 5
Introduced in #940
Furthermore, AWS returns also Scope and  SessionTimeout parameters that are filled with default values if not requested.
'Scope': 'openid',
'SessionTimeout': 604800,

That make the module always returns a change, if they are not requested.
This fix does not break backwards compatibility, because the values are already set by aws, when not requested yet.
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
plugins/module_utils/elbv2.yml
ADDITIONAL INFORMATION


          - Conditions:
              - Field: host-header
                Values:
                  - some.tld
              - Field: path-pattern
                Values:
                  - "/admin/*"
            Actions:
              - Type: authenticate-oidc
                Order: 1
                AuthenticateOidcConfig:
                  Issuer: https://login.microsoftonline.com/32rw-ewad53te-ef/v2.0
                  AuthorizationEndpoint: https://login.microsoftonline.com/324re-dafs6-6tw/oauth2/v2.0/authorize
                  TokenEndpoint: https://login.microsoftonline.com/432535ez-rfes-32543ter/oauth2/v2.0/token
                  UserInfoEndpoint: https://graph.microsoft.com/oidc/userinfo
                  ClientId: fasgd-235463-fsgd-243
                  ClientSecret: "{{ lookup('onepassword', 'some cool secret', vault='some important vault') }}"
                  SessionCookieName: AWSELBAuthSessionCookie
                  OnUnauthenticatedRequest: authenticate
                  UseExistingClientSecret: True
              - TargetGroupName: "{{ some_tg }}"
                Type: forward
                Order: 2

Reviewed-by: Alina Buzachis
Reviewed-by: Mark Chappell
patchback bot pushed a commit that referenced this issue Feb 23, 2023
elbv2: respect UseExistingClientSecret

SUMMARY
Since amazon.aws 5.0.0, elb_application_lb runs into an exception, when using Type: authenticate-oidc in a rule, even when UseExistingClientSecret: True parameter is given. That works as expected with amazon.aws 4.x.x.
The logic gets broken in  #940
Basically AWS won't return both, UseExistingClientSecret and  ClientSecret.
But when requesting against boto3,  both parameters are mutually exclusive!
When the user set UseExistingClientSecret: True, the ClientSecret must be removed for the request.
When the user does not set UseExistingClientSecret or set it to False,  the UseExistingClientSecret must be included in the request.
While diving deeper, I've noticed a basic change detection problem for default values, that are not requested, but AWS will return them. I've summerized it in #1284
However, this PR does not target #1284, it just fixes the exception and restores the functionality and hotfix the change-detection only for Type: authenticate-oidc.
origin PR description

The error was: botocore.errorfactory.InvalidLoadBalancerActionException: An error occurred (InvalidLoadBalancerAction) when calling the ModifyRule operation: You must either specify a client secret or set UseExistingClientSecret to true

UseExistingClientSecret is not respected anymore since a.a 5
Introduced in #940
Furthermore, AWS returns also Scope and  SessionTimeout parameters that are filled with default values if not requested.
'Scope': 'openid',
'SessionTimeout': 604800,

That make the module always returns a change, if they are not requested.
This fix does not break backwards compatibility, because the values are already set by aws, when not requested yet.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
plugins/module_utils/elbv2.yml
ADDITIONAL INFORMATION

          - Conditions:
              - Field: host-header
                Values:
                  - some.tld
              - Field: path-pattern
                Values:
                  - "/admin/*"
            Actions:
              - Type: authenticate-oidc
                Order: 1
                AuthenticateOidcConfig:
                  Issuer: https://login.microsoftonline.com/32rw-ewad53te-ef/v2.0
                  AuthorizationEndpoint: https://login.microsoftonline.com/324re-dafs6-6tw/oauth2/v2.0/authorize
                  TokenEndpoint: https://login.microsoftonline.com/432535ez-rfes-32543ter/oauth2/v2.0/token
                  UserInfoEndpoint: https://graph.microsoft.com/oidc/userinfo
                  ClientId: fasgd-235463-fsgd-243
                  ClientSecret: "{{ lookup('onepassword', 'some cool secret', vault='some important vault') }}"
                  SessionCookieName: AWSELBAuthSessionCookie
                  OnUnauthenticatedRequest: authenticate
                  UseExistingClientSecret: True
              - TargetGroupName: "{{ some_tg }}"
                Type: forward
                Order: 2

Reviewed-by: Alina Buzachis
Reviewed-by: Mark Chappell
(cherry picked from commit c6906a3)
softwarefactory-project-zuul bot pushed a commit that referenced this issue Feb 23, 2023
[PR #1270/c6906a3f backport][stable-5] elbv2: respect UseExistingClientSecret

This is a backport of PR #1270 as merged into main (c6906a3).
SUMMARY
Since amazon.aws 5.0.0, elb_application_lb runs into an exception, when using Type: authenticate-oidc in a rule, even when UseExistingClientSecret: True parameter is given. That works as expected with amazon.aws 4.x.x.
The logic gets broken in  #940
Basically AWS won't return both, UseExistingClientSecret and  ClientSecret.
But when requesting against boto3,  both parameters are mutually exclusive!
When the user set UseExistingClientSecret: True, the ClientSecret must be removed for the request.
When the user does not set UseExistingClientSecret or set it to False,  the UseExistingClientSecret must be included in the request.
While diving deeper, I've noticed a basic change detection problem for default values, that are not requested, but AWS will return them. I've summerized it in #1284
However, this PR does not target #1284, it just fixes the exception and restores the functionality and hotfix the change-detection only for Type: authenticate-oidc.
origin PR description

The error was: botocore.errorfactory.InvalidLoadBalancerActionException: An error occurred (InvalidLoadBalancerAction) when calling the ModifyRule operation: You must either specify a client secret or set UseExistingClientSecret to true

UseExistingClientSecret is not respected anymore since a.a 5
Introduced in #940
Furthermore, AWS returns also Scope and  SessionTimeout parameters that are filled with default values if not requested.
'Scope': 'openid',
'SessionTimeout': 604800,

That make the module always returns a change, if they are not requested.
This fix does not break backwards compatibility, because the values are already set by aws, when not requested yet.
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
plugins/module_utils/elbv2.yml
ADDITIONAL INFORMATION


          - Conditions:
              - Field: host-header
                Values:
                  - some.tld
              - Field: path-pattern
                Values:
                  - "/admin/*"
            Actions:
              - Type: authenticate-oidc
                Order: 1
                AuthenticateOidcConfig:
                  Issuer: https://login.microsoftonline.com/32rw-ewad53te-ef/v2.0
                  AuthorizationEndpoint: https://login.microsoftonline.com/324re-dafs6-6tw/oauth2/v2.0/authorize
                  TokenEndpoint: https://login.microsoftonline.com/432535ez-rfes-32543ter/oauth2/v2.0/token
                  UserInfoEndpoint: https://graph.microsoft.com/oidc/userinfo
                  ClientId: fasgd-235463-fsgd-243
                  ClientSecret: "{{ lookup('onepassword', 'some cool secret', vault='some important vault') }}"
                  SessionCookieName: AWSELBAuthSessionCookie
                  OnUnauthenticatedRequest: authenticate
                  UseExistingClientSecret: True
              - TargetGroupName: "{{ some_tg }}"
                Type: forward
                Order: 2

Reviewed-by: Mark Chappell
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Sep 18, 2023
Rename KMS modules

SUMMARY
In line with the naming guidelines, rename aws_kms and aws_kms_info
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/modules/aws_kms.py
plugins/modules/aws_kms_info.py
plugins/modules/kms_key.py
plugins/modules/kms_key_info.py
ADDITIONAL INFORMATION

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Sep 18, 2023
Rename KMS modules

SUMMARY
In line with the naming guidelines, rename aws_kms and aws_kms_info
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/modules/aws_kms.py
plugins/modules/aws_kms_info.py
plugins/modules/kms_key.py
plugins/modules/kms_key_info.py
ADDITIONAL INFORMATION

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Oct 24, 2023
Rename KMS modules

SUMMARY
In line with the naming guidelines, rename aws_kms and aws_kms_info
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/modules/aws_kms.py
plugins/modules/aws_kms_info.py
plugins/modules/kms_key.py
plugins/modules/kms_key_info.py
ADDITIONAL INFORMATION

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Alina Buzachis <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug has_pr python3
Projects
None yet
Development

No branches or pull requests

3 participants