openshift / openshift-ansible

Install and config an OpenShift 3.x cluster
https://try.openshift.com
Apache License 2.0
2.18k stars 2.31k forks source link

openshift_control_plane : set_fact - "|failed expects to filter on a list of identity providers" #9756

Closed varesa closed 6 years ago

varesa commented 6 years ago

Description

With openshift_master_identity_providers=[{...}] in INI format inventory file, deploy_cluster.yml from release-3.10 (8bbe09a98d240f96e213680d39fa29691a5038ba) fails

Version
 # ansible --version
ansible 2.6.2
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /bin/ansible
  python version = 2.7.5 (default, Aug  4 2017, 00:39:18) [GCC 4.8.5 20150623 (Red Hat 4.8.5-16)]

 # git describe
openshift-ansible-3.10.27-2-154-ga8d56b0
Steps To Reproduce
  1. Add openshift_master_identity_providers=[{<...>}] to INI format inventory file
  2. Run ansible-playbook deploy_cluster.yml
Expected Results

Playbook is successfull

Observed Results
TASK [openshift_control_plane : set_fact] *************************************************************************************************************************************************************************************************************************************
fatal: [openshift-master1]: FAILED! => {"msg": "|failed expects to filter on a list of identity providers"}
fatal: [openshift-master2]: FAILED! => {"msg": "|failed expects to filter on a list of identity providers"}
Additional Information

OS:

 # cat /etc/redhat-release
CentOS Linux release 7.4.1708 (Core)

Inventory:

[OSEv3:children]
masters
etcd
nodes
new_nodes

[OSEv3:vars]
ansible_ssh_user=root

openshift_deployment_type=origin
openshift_disable_check=memory_availability,disk_availability

openshift_metrics_install_metrics=false
openshift_logging_install_logging=false

openshift_hosted_registry_storage_kind=glusterfs
openshift_hosted_registry_storage_glusterfs_endpoints=gluster-endpoints
openshift_hosted_registry_storage_glusterfs_path="openshift_registry"
openshift_hosted_registry_storage_glusterfs_ips=["10.0.133.10","10.0.133.20","10.0.133.30"]

openshift_master_cluster_method=native
openshift_master_cluster_hostname=openshift-master.tre.esav.fi
openshift_master_cluster_public_hostname=openshift-master.tre.esav.fi

openshift_master_default_subdomain=ocp.tre.esav.fi
openshift_use_openshift_sdn=true
os_sdn_network_plugin_name='redhat/openshift-ovs-multitenant'
openshift_master_external_ip_network_cidrs=['10.0.137.0/24']

openshift_master_identity_providers=[{"name": "ipa_ldap_provider", "challenge": true, "login": true, "kind": "LDAPPasswordIdentityProvider","attributes": { "id": ["dn"], "email": ["mail"], "name": ["cn"], "preferredUsername": ["uid"] },"bindDN": "uid=bind_openshift,cn=users,cn=accounts,dc=esav,dc=fi","bindPassword": "redacted","insecure": true,"url": "ldap://ipa.tre.esav.fi/cn=users,cn=accounts,dc=esav,dc=fi?uid"}]

openshift_enable_service_catalog=false

[masters]
openshift-master[1:2]

[etcd]
openshift-master[1:2]
openshift-etcd1

[nodes]
openshift-master[1:2] openshift_node_group_name='node-config-master'
openshift-infra[1:2]  openshift_node_group_name='node-config-infra'
openshift-node[1:2]   openshift_node_group_name='node-config-compute'

[new_nodes]

I did some additional debugging:

diff --git a/roles/lib_utils/filter_plugins/openshift_master.py b/roles/lib_utils/filter_plugins/openshift_master.py
index 88b4bb2..6289f8b 100644
--- a/roles/lib_utils/filter_plugins/openshift_master.py
+++ b/roles/lib_utils/filter_plugins/openshift_master.py
@@ -464,6 +464,9 @@ class FilterModule(object):
         ''' Translates a list of dictionaries into a valid identityProviders config '''
         idp_list = []

+        print(type(idps))
+        print(idps)
+
         if not isinstance(idps, list):
             raise errors.AnsibleFilterError("|failed expects to filter on a list of identity providers")
         for idp in idps:

Result:

TASK [openshift_control_plane : set_fact] ********************************************************************************************
**************************************************************************************************************************************
***
<type 'unicode'>
[{"name": "ipa_ldap_provider", "challenge": true, "login": true, "kind": "LDAPPasswordIdentityProvider","attributes": { "id": ["dn"],
"email": ["mail"], "name": ["cn"], "preferredUsername": ["uid"] },"bindDN": "uid=bind_openshift,cn=users,cn=accounts,dc=esav,dc=fi","b
indPassword": "redacted","insecure": true,"url": "ldap://ipa.tre.esav.fi/cn=users,cn=accounts,dc=esav,dc=fi?uid"}]
fatal: [openshift-master1]: FAILED! => {"msg": "|failed expects to filter on a list of identity providers"}
<type 'unicode'>
[{"name": "ipa_ldap_provider", "challenge": true, "login": true, "kind": "LDAPPasswordIdentityProvider","attributes": { "id": ["dn"],
"email": ["mail"], "name": ["cn"], "preferredUsername": ["uid"] },"bindDN": "uid=bind_openshift,cn=users,cn=accounts,dc=esav,dc=fi","b
indPassword": "redacted","insecure": true,"url": "ldap://ipa.tre.esav.fi/cn=users,cn=accounts,dc=esav,dc=fi?uid"}]
fatal: [openshift-master2]: FAILED! => {"msg": "|failed expects to filter on a list of identity providers"}
michaelgugino commented 6 years ago

While very subtle, ansible is very picky about how things are defined in the ini inventory.

Your usage of double-quotes inside your list of dictionaries in place of single quotes results in ansible not recognizing the value as a list of of dictionaries, rather a string (or possibly, a list of strings).

Ensure you use single quotes as in the example.

varesa commented 6 years ago

@michaelgugino Thanks for the fix. (I'll confirm in a sec)

However if double quotes should be avoided, they should be removed from the examples: #openshift_master_identity_providers=[{"name": "openid_auth", "login": "true", "challenge": "false", "kind": "OpenIDIdentityProvider", "client_id": "my_client_id", "client_secret": "my_client_secret", "claims": {"id": ["sub"], "preferredUsername": ["preferred_username"], "name": ["name"], "email": ["email"]}, "urls": {"authorize": "https://myidp.example.com/oauth2/authorize", "token": "https://myidp.example.com/oauth2/token"}, "ca": "my-openid-ca-bundle.crt"}] https://github.com/openshift/openshift-ansible/blob/master/inventory/hosts.example#L225

#openshift_master_identity_providers=[{"name": "my_request_header_provider", "challenge": "true", "login": "true", "kind": "RequestHeaderIdentityProvider", "challengeURL": "https://www.example.com/challenging-proxy/oauth/authorize?${query}", "loginURL": "https://www.example.com/login-proxy/oauth/authorize?${query}", "clientCA": "my-request-header-ca.crt", "clientCommonNames": ["my-auth-proxy"], "headers": ["X-Remote-User", "SSO-User"], "emailHeaders": ["X-Remote-User-Email"], "nameHeaders": ["X-Remote-User-Display-Name"], "preferredUsernameHeaders": ["X-Remote-User-Login"]}] https://github.com/openshift/openshift-ansible/blob/master/inventory/hosts.example#L239

michaelgugino commented 6 years ago

@varesa Thanks for pointing that out. I didn't notice those two. I'll get a patch out (unless you want to do it).

varesa commented 6 years ago

@michaelgugino I wouldn't mind getting a contribution in.

Looks like (at least) these should all be changed, no?

esa@desktop ~/openshift-ansible/inventory (fix_inventory_quotes) $ grep '=[{[].*"' *
hosts.example:#openshift_master_image_policy_config={"maxImagesBulkImportedPerRepository": 3, "disableScheduledImport": true}
hosts.example:#openshift_master_image_policy_allowed_registries_for_import=["docker.io", "*.docker.io", "*.redhat.com", "gcr.io", "quay.io", "registry.centos.org", "registry.redhat.io", "*.amazonaws.com"]
hosts.example:#openshift_master_identity_providers=[{"name": "openid_auth", "login": "true", "challenge": "false", "kind": "OpenIDIdentityProvider", "client_id": "my_client_id", "client_secret": "my_client_secret", "claims": {"id": ["sub"], "preferredUsername": ["preferred_username"], "name": ["name"], "email": ["email"]}, "urls": {"authorize": "https://myidp.example.com/oauth2/authorize", "token": "https://myidp.example.com/oauth2/token"}, "ca": "my-openid-ca-bundle.crt"}]
hosts.example:#openshift_master_identity_providers=[{"name": "my_request_header_provider", "challenge": "true", "login": "true", "kind": "RequestHeaderIdentityProvider", "challengeURL": "https://www.example.com/challenging-proxy/oauth/authorize?${query}", "loginURL": "https://www.example.com/login-proxy/oauth/authorize?${query}", "clientCA": "my-request-header-ca.crt", "clientCommonNames": ["my-auth-proxy"], "headers": ["X-Remote-User", "SSO-User"], "emailHeaders": ["X-Remote-User-Email"], "nameHeaders": ["X-Remote-User-Display-Name"], "preferredUsernameHeaders": ["X-Remote-User-Login"]}]
hosts.example:#openshift_hosted_router_certificate={"certfile": "/path/to/router.crt", "keyfile": "/path/to/router.key", "cafile": "/path/to/router-ca.crt"}
hosts.example:#openshift_master_named_certificates=[{"certfile": "/path/to/custom1.crt", "keyfile": "/path/to/custom1.key", "cafile": "/path/to/custom-ca1.crt"}]
hosts.example:#openshift_master_named_certificates=[{"certfile": "/path/to/custom1.crt", "keyfile": "/path/to/custom1.key", "names": ["public-master-host.com"], "cafile": "/path/to/custom-ca1.crt"}]
hosts.example:#logrotate_scripts=[{"name": "syslog", "path": "/var/log/cron\n/var/log/maillog\n/var/log/messages\n/var/log/secure\n/var/log/spooler\n", "options": ["daily", "rotate 7", "compress", "sharedscripts", "missingok"], "scripts": {"postrotate": "/bin/kill -HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true"}}]
hosts.example:#openshift_master_admission_plugin_config={"ProjectRequestLimit":{"configuration":{"apiVersion":"v1","kind":"ProjectRequestLimitConfig","limits":[{"selector":{"admin":"true"}},{"maxProjects":"1"}]}},"PodNodeConstraints":{"configuration":{"apiVersion":"v1","kind":"PodNodeConstraintsConfig"}}}
hosts.example:#openshift_node_env_vars={"ENABLE_HTTP2": "true"}
hosts.example:#openshift_master_audit_config={"enabled": "true"}
hosts.example:#openshift_master_audit_config={"enabled": "true", "auditFilePath": "/var/lib/origin/openpaas-oscp-audit/openpaas-oscp-audit.log", "maximumFileRetentionDays": "14", "maximumFileSizeMegabytes": "500", "maximumRetainedFiles": "5"}
hosts.example:#openshift_master_open_ports=[{"service":"svc1","port":"11/tcp"}]
hosts.example:#openshift_node_open_ports=[{"service":"svc2","port":"12-13/tcp"},{"service":"svc3","port":"14/udp"}]
varesa commented 6 years ago

Actually replacing double quotes with singles did not fix the issue:

openshift-master1 playbooks (release-3.10) # cat /etc/ansible/hosts | grep identity
openshift_master_identity_providers=[{'name': 'ipa_ldap_provider', 'challenge': true, 'login': true, 'kind': 'LDAPPasswordIdentityProvider','attributes': { 'id': ['dn'], 'email': ['mail'], 'name': ['cn'], 'preferredUsername': ['uid'] },'
bindDN': 'uid=bind_openshift,cn=users,cn=accounts,dc=esav,dc=fi','bindPassword': 'redact','insecure': true,'url': 'ldap://ipa.tre.esav.fi/cn=users,cn=accounts,dc=esav,dc=fi?uid'}]
TASK [openshift_control_plane : set_fact] ***************************************************************************************************************************************************************************************************
fatal: [openshift-master1]: FAILED! => {"msg": "|failed expects to filter on a list of identity providers"}
fatal: [openshift-master2]: FAILED! => {"msg": "|failed expects to filter on a list of identity providers"}
varesa commented 6 years ago

This seems to actually be caused by a change in ansible:

https://github.com/ansible/ansible/commit/c3550b58ede1fcfaf84da2a81bab394e040802bd (documentation only)

From:

Values passed in using the key=value syntax are interpreted as Python literal structure (strings, numbers, tuples, lists, dicts, booleans, None), alternatively as string.

To:

Values passed in the INI format using the key=value syntax are not interpreted as Python literal structure (strings, numbers, tuples, lists, dicts, booleans, None), but as a string.

The commit (documentation change) is contained in ansible branches stable-2.5 and stable-2.6 (not 2.4)

varesa commented 6 years ago

Ansible >= 2.4.3.0, 2.5.x is not currently supported for OCP installations

I see now that 2.5 (and I'd assume 2.6?) is not supported by the release-3.10 branch (earlier I got a recommendation to upgrade to >= 2.5 while upgrading 3.9->3.10 on #openshift-dev but never mind that).

However the master branch still has those examples and states a requirement of ansible >= 2.6.2 which makes me think this issue is still relevant (although the fix is not as simple as s/"/'/g)

michaelgugino commented 6 years ago

@varesa thanks for pointing this out. We'll definitely need to clean some things up if this is the case. @mtnbikenc @sdodson do you know if the above ansible commit is accurate?

sdodson commented 6 years ago

Yes, master branch requires >= 2.6.2

csheremeta commented 6 years ago

Hi @varesa @michaelgugino @sdodson I have a customer who is hitting this issue. I opened https://bugzilla.redhat.com/show_bug.cgi?id=1626573 for it.

Thanks!

varesa commented 6 years ago

@csheremeta FYI I came up with two workarounds,

first one I did while debugging/testing the issue. In roles/lib_utils/filter_plugins/openshift_master.py I added

import json
idps = json.loads(idps)

as long as the inventory string is valid json (double quotes, etc.) it parses it to a list which allows it to run nicely.

A more "proper" workaround was to move the openshift_master_identity_providers-config from the INI file into a YAML file in /etc/ansible/group_vars/OSEv3.yml

Just make the file like this:

openshift_master_identity_providers:
- name: your_provider
  challenge: true
  login: true
  kind: ...
  restOf: properties
sdodson commented 6 years ago

The only reasonable path forward is to specify these variables in YAML format as described in https://github.com/openshift/openshift-ansible/issues/9756#issuecomment-419518998. Specifying them in INI formatted inventory is no longer possible starting with Ansible 2.4.