Bug 1670473 - The variable openshift_master_image_policy_allowed_registries_for_import is not being validated correctly by ansible
Summary: The variable openshift_master_image_policy_allowed_registries_for_import is n...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: Installer
Version: 3.11.0
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
: 3.11.z
Assignee: Vadim Rutkovsky
QA Contact: Gaoyun Pei
URL:
Whiteboard:
: 1646207 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-01-29 15:38 UTC by Oscar Casal Sanchez
Modified: 2019-04-11 05:38 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Cause: openshift_master_image_policy_allowed_registries_for_import was incorrectly parsed Consequence: a corrupted master-config.yaml was generated when openshift_master_image_policy_allowed_registries_for_import was used Fix: openshift_master_image_policy_allowed_registries_for_import is being correctly parsed Result: a simple registry image policy can be set using openshift_master_image_policy_allowed_registries_for_import
Clone Of:
Environment:
Last Closed: 2019-04-11 05:38:26 UTC
Target Upstream Version:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Knowledge Base (Solution) 3881161 0 Install None Could not load config file "/etc/origin/master/master-config.yaml" due to an error reading config 2019-03-05 19:02:06 UTC
Red Hat Product Errata RHBA-2019:0636 0 None None None 2019-04-11 05:38:35 UTC

Internal Links: 1670483

Description Oscar Casal Sanchez 2019-01-29 15:38:07 UTC
Description of problem:
#######################
As a user, I follow the /usr/share/doc/openshift-ansible-docs-3.11.59/docs/host.example/host.example file delivered with the openshift-ansible-docs-3.11.59-1.git.0.ba8e948.el7.noarch package would use the following line:

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"]

When the cluster is deployed, this line is creating invalid entries at /etc/origin/master/master-config.yml like this:

...
imagePolicyConfig:
  allowedRegistriesForImport:
  - docker.io
  - '*.docker.io'
  - '*.redhat.com'
  - gcr.io
  - quay.io
  - registry.centos.org
  - registry.redhat.io
  - '*.amazonaws.com'
...

When it should be something like this:

imagePolicyConfig:
  imagePolicyConfig:
    allowedRegistriesForImport:
    - domainName: docker.io
    - domainName: '*.docker.io'
    - domainName: '*.redhat.com'
    - ...
...

Ansible is not checking that the openshift_master_image_policy_allowed_registries_for_import is a dictionary with 'domainName' key.


Version-Release number of the following components:
###################################################

$ rpm -q openshift-ansible
openshift-ansible-3.11.59-1.git.0.ba8e948.el7.noarch

$ rpm -q ansible
ansible-2.6.12-1.el7ae.noarch

$ ansible --version
ansible 2.6.12
  config file = /usr/share/ansible/openshift-ansible/ansible.cfg
  configured module search path = [u'/home/quicklab/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Sep 12 2018, 05:31:16) [GCC 4.8.5 20150623 (Red Hat 4.8.5-36)]

How reproducible:
#################
Always

Steps to Reproduce:
###################
1. Use the 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"] entry line delivered with the /usr/share/doc/openshift-ansible-docs-3.11.59/docs/host.example/host.example file delivered with the openshift-ansible-docs-3.11.59-1.git.0.ba8e948.el7.noarch package to create the ansible host file
2. Execute ansible-playbook -i [hosts] /usr/share/ansible/openshift-ansible/playbooks/deploy_cluster.yml
3. Check the content of the file /etc/origin/master/master-config.yaml when the ansible-playbook command has finished (with fail or not state) where you can see the next:


Actual results:
###############

When the cluster is deployed, the /etc/origin/master/master-config.yml is like this:

...
imagePolicyConfig:
  allowedRegistriesForImport:
  - docker.io
  - '*.docker.io'
  - '*.redhat.com'
  - gcr.io
  - quay.io
  - registry.centos.org
  - registry.redhat.io
  - '*.amazonaws.com'
...

Expected results:
#################

When the cluster is deployed, the /etc/origin/master/master-config.yml should be like this:

...
imagePolicyConfig:
  imagePolicyConfig:
    allowedRegistriesForImport:
    - domainName: docker.io
    - domainName: '*.docker.io'
    - domainName: '*.redhat.com'
    - ...
...

And if as a customer I define a line like this:

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"]

I hope that ansible throws an error indicating that it should be a must be a dictionary with 'domainName' key" or similar when it executes the /usr/share/ansible/openshift-ansible/roles/lib_utils/action_plugins/sanity_checks.py. In this file this is checked when "allowed_registries_for_import is None":

...
        allowed_registries_for_import = self.template_var(hostvars, host, ALLOWED_REGISTRIES_VAR)
        if allowed_registries_for_import is None:
...
            try:
                return [i["domainName"] for i in detailed]
            except Exception:
                raise errors.AnsibleModuleError(
                    "each item of allowedRegistriesForImport must be a dictionary with 'domainName' key")

But, it's not validated when "allowed_registries_for_import" is not None. That it's the case when we define the variable openshift_master_image_policy_allowed_registries_for_import in our ansible host file.


Additional info:

Comment 1 Oscar Casal Sanchez 2019-02-04 14:08:40 UTC
Hi,

From the description of the problem, I have made a mistake with this lines:

---
When it should be something like this:

imagePolicyConfig:
  imagePolicyConfig:
    allowedRegistriesForImport:
    - domainName: docker.io
    - domainName: '*.docker.io'
    - domainName: '*.redhat.com'
    - ...
---

But it should be something like this:


---
When it should be something like this:

imagePolicyConfig:
  allowedRegistriesForImport:
  - domainName: docker.io
  - domainName: '*.docker.io'
  - domainName: '*.redhat.com'
  - ...
---

Comment 2 Brenton Leanhardt 2019-03-05 16:26:07 UTC
Hi Oscar,

I'm not 100% certain I'm following you.  In https://github.com/openshift/openshift-ansible/blob/release-3.11/roles/openshift_facts/library/openshift_facts.py#L500 you can see that, internally, openshift ansible creates the necessary dictionary for use in the fact checking.  This is simply an internal representation of the configuration that users can specify.  It sounds like you are expecting users to have to set 'domainName' as a key.  This should be getting set automatically.  If it's not, then I would expect that's where the bug is.  I'll ask someone more knowledgeable in the code to chime in.

Comment 3 Vadim Rutkovsky 2019-03-05 16:36:19 UTC
`openshift_master_image_policy_allowed_registries_for_import` is expected to be a simple list of registries, it won't allow `insecure` to be set for some.

A more complex structure of image policy settings should be set via openshift_master_image_policy_config.

I'd propose to close this as a WONTFIX, as the functionality is already provided by openshift_master_image_policy_config - and changing format in openshift_master_image_policy_allowed_registries_for_import would break existing inventories

Comment 4 Oscar Casal Sanchez 2019-03-05 16:46:09 UTC
Hi Brenton,

The example provided by us in /usr/share/doc/openshift-ansible-docs-3.11.59/docs/host.example/host.example is this:

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"]

With this line, the result is this:

$ cat /etc/origin/master/master-config.yml like this:

...
imagePolicyConfig:
  allowedRegistriesForImport:
  - docker.io
  - '*.docker.io'
  - '*.redhat.com'
  - gcr.io
  - quay.io
  - registry.centos.org
  - registry.redhat.io
  - '*.amazonaws.com'

And this result is not a valid format. 

The workaround provided was this line:

openshift_master_image_policy_allowed_registries_for_import=[{"domainName":"*.redhat.com"},{"domainName":"quay.io"},{"domainName":"registry.redhat.io"}...]

And that generated the next /etc/origin/master/master-config.yml that is ok:

imagePolicyConfig:
  allowedRegistriesForImport:
  - domainName: docker.io
  - domainName: '*.docker.io'
  - domainName: '*.redhat.com'

As you are saying, the user needs to set the domainName as a key

Comment 5 Brenton Leanhardt 2019-03-05 16:50:47 UTC
Vadim actually found another bug where it seems the same problem is hit.  Could you provide playbook -vvv output, inventory, and contents of master's config?

https://bugzilla.redhat.com/show_bug.cgi?id=1670473

Comment 6 Brenton Leanhardt 2019-03-05 16:54:46 UTC
*** Bug 1646207 has been marked as a duplicate of this bug. ***

Comment 8 Oscar Casal Sanchez 2019-03-05 19:02:06 UTC
Hello Brenton,

With this line in the ansible host:

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"]

The installer failed and you could see the next error:

$ master-logs api api        
F0125 13:31:23.925985       1 start_api.go:68] could not load config file "/etc/origin/master/master-config.yaml" due to an error: error reading config: v1.MasterConfig.ImagePolicyConfig: v1.ImagePolicyConfig.AllowedRegistriesForImport: v1.AllowedRegistries: readObjectStart: expect { or n, but found ", error found in #10 byte of ...|Import":["*.redhat.c|..., bigger context ...|magePolicyConfig":{"allowedRegistriesForImport":["*.redhat.com","quay.io","registry.redhat.io","*.sc|...

And how I described in the description of the bugzilla the /etc/origin/master/master-config.yml will look like this:

$ cat /etc/origin/master/master-config.yml
...
imagePolicyConfig:
  allowedRegistriesForImport:
  - docker.io
  - '*.docker.io'
  - '*.redhat.com'
  - gcr.io
  - quay.io
  - registry.centos.org
  - registry.redhat.io
  - '*.amazonaws.com'
...

When It should be something like this:

$ cat /etc/origin/master/master-config.yml

imagePolicyConfig:
  allowedRegistriesForImport:
  - domainName: docker.io
  - domainName: '*.docker.io'
  - domainName: '*.redhat.com'
  - ...


Then, the installation fails and it's always reproducible.

Comment 9 Vadim Rutkovsky 2019-03-08 11:16:48 UTC
The check for string values in this list was incorrect, created PR https://github.com/openshift/openshift-ansible/pull/11327 to fix it

Comment 10 Oscar Casal Sanchez 2019-03-08 11:41:04 UTC
Thank you so much Vadim, Scott and Brenton

Comment 11 Vadim Rutkovsky 2019-03-12 10:39:43 UTC
Fix is available in openshift-ansible-3.11.94-1

Comment 12 Gaoyun Pei 2019-03-13 08:58:15 UTC
Could reproduce this bug with openshift-ansible-3.11.69-1.git.0.2ff281f.el7.noarch.rpm

Set 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"] in ansible inventory file, installation failed when starting master service

[root@ip-172-18-6-231 ~]# master-logs api api
F0313 08:29:37.830878       1 start_api.go:68] could not load config file "/etc/origin/master/master-config.yaml" due to an error: error reading config: v1.MasterConfig.ImagePolicyConfig: v1.ImagePolicyConfig.AllowedRegistriesForImport: v1.AllowedRegistries: readObjectStart: expect { or n, but found ", error found in #10 byte of ...|Import":["docker.io"|..., bigger context ...|magePolicyConfig":{"allowedRegistriesForImport":["docker.io","*.docker.io","*.redhat.com","gcr.io","|...

[root@ip-172-18-6-231 ~]# grep -A 10 allowedRegistriesForImport /etc/origin/master/master-config.yaml
  allowedRegistriesForImport:
  - docker.io
  - '*.docker.io'
  - '*.redhat.com'
  - gcr.io
  - quay.io
  - registry.centos.org
  - registry.redhat.io
  - '*.amazonaws.com'
  internalRegistryHostname: docker-registry.default.svc:5000
kind: MasterConfig



Verified this bug with openshift-ansible-3.11.95-1.git.0.d080cce.el7.noarch.rpm

1) With incorrect openshift_master_image_policy_allowed_registries_for_import set in ansible inventory file
openshift_master_image_policy_allowed_registries_for_import=["docker.io", {"domainName":"registry.redhat.io""}]


TASK [Run variable sanity checks] **********************************************
Wednesday 13 March 2019  16:28:24 +0800 (0:00:00.726)       0:01:04.483 ******* 
fatal: [ec2-54-152-147-133.compute-1.amazonaws.com]: FAILED! => {"msg": "last_checked_host: ec2-54-152-147-133.compute-1.amazonaws.com, last_checked_var: openshift_master_image_policy_allowed_registries_for_import;expected list for openshift_master_image_policy_config, not <type 'unicode'>"}
	to retry, use: --limit @openshift-ansible/playbooks/prerequisites.retry


2) With a correct list
openshift_master_image_policy_allowed_registries_for_import=["docker.io", "*.docker.io", "*.redhat.com", "gcr.io", "quay.io", "registry.centos.org", "registry.redhat.io", "*.openshift.com"]

Installation finished without error.

[root@ip-172-18-4-134 ~]# grep -A 10 allowedRegistriesForImport /etc/origin/master/master-config.yaml
  allowedRegistriesForImport:
  - domainName: docker.io
  - domainName: '*.docker.io'
  - domainName: '*.redhat.com'
  - domainName: gcr.io
  - domainName: quay.io
  - domainName: registry.centos.org
  - domainName: registry.redhat.io
  - domainName: '*.openshift.com'
  internalRegistryHostname: docker-registry.default.svc:5000
kind: MasterConfig

Comment 14 errata-xmlrpc 2019-04-11 05:38:26 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2019:0636


Note You need to log in before you can comment on or make changes to this bug.