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

validate_certs defaulting to true is a breaking change #357

Closed
MallocArray opened this issue Jan 27, 2022 · 7 comments · Fixed by #361
Closed

validate_certs defaulting to true is a breaking change #357

MallocArray opened this issue Jan 27, 2022 · 7 comments · Fixed by #361

Comments

@MallocArray
Copy link

Summary

In the 5.0.0 release, it was noted that all modules support SSL over HTTPS, but did not make clear that validate_certs now defaults to true on modules. In addition, when validate_certs is true, it REQUIRES ca_path to be filled out or the task will fail. This is ultimately a breaking change as playbooks that worked without specifying validate_certs in the past now do not work at all with an error:

FAILED! => {"ansible_facts": {"discovered_interpreter_python": "/usr/libexec/platform-python"}, "changed": false, "msg": "validate_certs is True but all of the following are missing: ca_path"}

I have modules for other technologies that I do use validate_certs with, but they do not require a ca_path field to be filled out. Why do the Dell modules require this instead of using a default path or autodiscovering?

At this point, in order to use 5.0.0 I either have to modify all tasks to change validate_certs: false or modify all tasks to include a ca_path value, both of which will take unexpected time and effort.

Component Name

ome_application_network_time and presumably all other modules

Ansible Version
ansible 2.10.16.post0
  config file = /runner/ansible.cfg
  configured module search path = ['/home/runner/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.8/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.8.8 (default, Aug 25 2021, 16:13:02) [GCC 8.5.0 20210514 (Red Hat 8.5.0-3)]
iDRAC or OpenManage Enterprise version

OME-M 1.40.10 and 1.30.10

Steps to Reproduce

Have working playbooks without validate_certs specified
Upgrade to collection 5.0.0
Experience failures because validate_certs defaults to true, but ca_path has no default value

Playbook used
  tasks:
    - name: Configure NTP server for time synchronization.
      dellemc.openmanage.ome_application_network_time:
        hostname: "{{ inventory_hostname }}"
        username: "{{ dell_ome_username }}"
        password: "{{ dell_ome_password }}"
        time_zone: "{{ ntp_config.time_zone }}"
        enable_ntp: true
        primary_ntp_address: "{{ ntp_config.primary }}"
        secondary_ntp_address1: "{{ ntp_config.secondary1 }}"
        secondary_ntp_address2: "{{ ntp_config.secondary2 }}"
      tags:
        - ntp
Expected Results

When validate_certs is defaulted to true, it would not require an additional parameter of ca_path and instead use some default value that other collections must be using to support cert validation without needing to specify a ca_path, as I don't even know what path I would need to use in my Ansible Execution Environment

Actual Results

Community Note

  • Please vote on this issue by adding a 👍 reaction
    to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions,
    they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@anupamaloke
Copy link
Collaborator

@MallocArray, I will check internally and get back on this on whether we will revert the default to False. For now, I would recommend to make the default value as False for the validate_certs module argument.

@howezies
Copy link

howezies commented Jan 31, 2022

I am getting an error on the dellemc.openmanage.dellemc_configure_idrac_services module. I now have validate_certs: false but get this error:
"msg": "Unable to communicate with iDRAC 10.0.0.0. This may be due to one of the following: Incorrect username or password, unreachable iDRAC IP or a failure in TLS/SSL handshake."

Other modules within the dellemc.openmanage collection work fine.

@jagadeeshnv
Copy link
Contributor

@howezies update omsdk to latest, if issue still persists, open a separate issue for the same, we will look into it

@anupamaloke
Copy link
Collaborator

@MallocArray, I had a discussion internally. There were suggestions on keeping the validate_certs to False initially, but later due to mandatory security requirements, it was decided to keep this as True by default. I agree that this is a breaking change in the sense that you will have to modify each and every task in your playbook to add this new argument.

I need to discuss this further with the team on how we could ease the transition or make it tied to say for e.g. environment variables that will provide a default values for these new arguments.

@MallocArray
Copy link
Author

I'm mostly concerned with these 2 points:

  1. Why was this major of a change not called out in the release notes? It does say that it supports SSL over HTTPS, but not that it now defaults to true or that a new parameter ca_path is required. Either way, you have to update every task because either you have to set validate_certs: false or 'ca_path: whatever' and I can't think of a circumstance where someone could upgrade from 4.x to 5.x without adding one of those two, so it should be a big huge bold heading of the release notes that you must change something for anything to work.
  2. This is by far my biggest concern, why is ca_path required at all? I checked two different VMware collections which both have a validate_certs parameter that defaults to true, but no path to a cert is required. I assume it uses whatever the ansible host has and it validates my certs properly. With the new Dell collection, I must give a ca_path that I have no idea what to provide or why. All of my MX7000 chassis have a valid cert installed, so they should just work with validate_certs: true if the path wasn't a requirement.

@ghost
Copy link

ghost commented Feb 4, 2022

Ran into this issue today as well, had to downgrade to 4.4.0 to get my work done.

@sachin-apa
Copy link
Collaborator

sachin-apa commented Feb 10, 2022

All, we are having a patch release 5.0.1 targeted for 11 Feb 2022 with the following changes.

  • Based on the security team recommendations we need to ensure all the modules validates SSL certificates by default and hence validate_certs will remain True by default and users need to explicitly set it to False if SSL validation has to be disabled.
  • If users choose to validate SSL, we are introducing the environment variable to provide a certificate path instead of expecting a mandatory parameter from the playbook.
  • All the steps and info on the changes that needs to be done as part of the SSL will be updated in the readme.
    @anupamaloke

@sachin-apa sachin-apa linked a pull request Feb 11, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants