Ensure Clear Linux parsing is actually parsing a Clear Linux host and all others fall back to NA #53298

samdoran · 2019-03-04T22:40:23Z

SUMMARY

Fixes a bug where parse_distribution_file_ClearLinux() was called on CoreOS (and probably many other distros that had /etc/os-release) and it returned True since it successfully parsed the distribution file and there was no check of the NAME field to ensure it was actually a Clear Linux host.

Change the order in which distribution files are processed so NA is last. This prevents a match on CoreOS hosts since they also have /etc/os-release and the last item in OSDIST_LIST wins.

Fixes #50009
Depends on #52199

Before this change:

> ansible lab-coreos,lab-clear,lab-mint -m setup -a 'filter=ansible_distribution*'

lab-coreos | SUCCESS => {
    "ansible_facts": {
        "ansible_distribution": "Container Linux by CoreOS",
        "ansible_distribution_file_parsed": true,
        "ansible_distribution_file_path": "/usr/lib/os-release",
        "ansible_distribution_file_variety": "ClearLinux",
        "ansible_distribution_major_version": "1911.5.0",
        "ansible_distribution_release": "coreos",
        "ansible_distribution_version": "1911.5.0"
    },
    "changed": false
}
lab-clear | SUCCESS => {
    "ansible_facts": {
        "ansible_distribution": "Clear Linux OS",
        "ansible_distribution_file_parsed": true,
        "ansible_distribution_file_path": "/usr/lib/os-release",
        "ansible_distribution_file_variety": "ClearLinux",
        "ansible_distribution_major_version": "28120",
        "ansible_distribution_release": "clear-linux-os",
        "ansible_distribution_version": "28120"
    },
    "changed": false
}

lab-mint | SUCCESS => {
    "ansible_facts": {
        "ansible_distribution": "Linux Mint",
        "ansible_distribution_file_parsed": true,
        "ansible_distribution_file_path": "/etc/os-release",
        "ansible_distribution_file_variety": "Debian",
        "ansible_distribution_major_version": "19",
        "ansible_distribution_release": "tessa",
        "ansible_distribution_version": "19.1",
        "discovered_interpreter_python": "/usr/bin/python"
    },
    "changed": false
}

After this change:

> ansible lab-coreos,lab-clear,lab-mint -m setup -a 'filter=ansible_distribution*'

lab-coreos | SUCCESS => {
    "ansible_facts": {
        "ansible_distribution": "Coreos",
        "ansible_distribution_file_parsed": true,
        "ansible_distribution_file_path": "/etc/coreos/update.conf",
        "ansible_distribution_file_variety": "Coreos",
        "ansible_distribution_major_version": "1911",
        "ansible_distribution_release": "stable",
        "ansible_distribution_version": "1911.5.0"
    },
    "changed": false
}
lab-clear | SUCCESS => {
    "ansible_facts": {
        "ansible_distribution": "Clear Linux OS",
        "ansible_distribution_file_parsed": true,
        "ansible_distribution_file_path": "/usr/lib/os-release",
        "ansible_distribution_file_variety": "ClearLinux",
        "ansible_distribution_major_version": "28120",
        "ansible_distribution_release": "clear-linux-os",
        "ansible_distribution_version": "28120"
    },
    "changed": false
}

lab-mint | SUCCESS => {
    "ansible_facts": {
        "ansible_distribution": "Linux Mint",
        "ansible_distribution_file_parsed": true,
        "ansible_distribution_file_path": "/etc/os-release",
        "ansible_distribution_file_variety": "Debian",
        "ansible_distribution_major_version": "19",
        "ansible_distribution_release": "tessa",
        "ansible_distribution_version": "19.1",
        "discovered_interpreter_python": "/usr/bin/python"
    },
    "changed": false
}

The change is subtle: ansible_distribution_file_variety is now correct, which means we used the correct distribution file parsing method.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

lib/ansible/module_utils/facts/system/distribution.py

ADDITIONAL INFORMATION

Here is /etc/os-release from Linux Mint, Clear Linux, and CoreOS:


lab-coreos | CHANGED | rc=0 >>
NAME="Container Linux by CoreOS"
ID=coreos
VERSION=1911.5.0
VERSION_ID=1911.5.0
BUILD_ID=2018-12-15-2317
PRETTY_NAME="Container Linux by CoreOS 1911.5.0 (Rhyolite)"
ANSI_COLOR="38;5;75"
HOME_URL="https://coreos.com/"
BUG_REPORT_URL="https://issues.coreos.com"
COREOS_BOARD="amd64-usr"

lab-mint | CHANGED | rc=0 >>
NAME="Linux Mint"
VERSION="19.1 (Tessa)"
ID=linuxmint
ID_LIKE=ubuntu
PRETTY_NAME="Linux Mint 19.1"
VERSION_ID="19.1"
HOME_URL="https://www.linuxmint.com/"
SUPPORT_URL="https://forums.ubuntu.com/"
BUG_REPORT_URL="http://linuxmint-troubleshooting-guide.readthedocs.io/en/latest/"
PRIVACY_POLICY_URL="https://www.linuxmint.com/"
VERSION_CODENAME=tessa
UBUNTU_CODENAME=bionic

lab-clear | CHANGED | rc=0 >>
NAME="Clear Linux OS"
VERSION=1
ID=clear-linux-os
ID_LIKE=clear-linux-os
VERSION_ID=28120
PRETTY_NAME="Clear Linux OS"
ANSI_COLOR="1;35"
HOME_URL="https://clearlinux.org"
SUPPORT_URL="https://clearlinux.org"
BUG_REPORT_URL="mailto:dev@lists.clearlinux.org"
PRIVACY_POLICY_URL="http://www.intel.com/privacy"

Fixes a bug where parse_distribution_file_ClearLinux() was called on CoreOS (and probably many other distros) and it returned True since it successfully parses the distribution file. Since this file exists on many Linux distributions and they are very similar format, add an additional check to make sure it is Clear Linux. Change the order in which distribution files are processed no NA is last. This prevents a match on CoreOS hosts since they also have /etc/os-release and the last item in OSDIST_LIST wins.

Akasurde · 2019-03-05T05:35:11Z

@samdoran Could you please add unit test for this change ? Thanks.

Only add tests for Clear Linux parsing since that was the cause of this issue.

maxamillion · 2019-03-07T17:19:30Z

shipit

Akasurde · 2019-03-08T14:10:39Z

shipit

…inux host and all others fall back to NA (ansible#53298) Fixes a bug where parse_distribution_file_ClearLinux() was called on CoreOS (and probably many other distros) and it returned True since it successfully parses the distribution file. Since this file exists on many Linux distributions and they are a very similar format, add an additional check to make sure it is Clear Linux. Change the order in which distribution files are processed so NA is last. This prevents a match on CoreOS hosts since they also have /etc/os-release and the called matching function for NA is very general and will match CoreOS. * Add changelog * Add unit tests Only add tests for Clear Linux parsing since that was the cause of this issue. (cherry picked from commit 1d91e03) Co-authored-by: Sam Doran <sdoran@redhat.com>

…inux host and all others fall back to NA (#53298) (#53541) * [stable-2.7] Ensure Clear Linux parsing is actually parsing a Clear Linux host and all others fall back to NA (#53298) Fixes a bug where parse_distribution_file_ClearLinux() was called on CoreOS (and probably many other distros) and it returned True since it successfully parses the distribution file. Since this file exists on many Linux distributions and they are a very similar format, add an additional check to make sure it is Clear Linux. Change the order in which distribution files are processed so NA is last. This prevents a match on CoreOS hosts since they also have /etc/os-release and the called matching function for NA is very general and will match CoreOS. * Add changelog * Add unit tests Only add tests for Clear Linux parsing since that was the cause of this issue. (cherry picked from commit 1d91e03) Co-authored-by: Sam Doran <sdoran@redhat.com> * Use different import for 2.7

samdoran added 2 commits March 4, 2019 17:11

Add changelog

09861bc

samdoran requested a review from maxamillion March 4, 2019 22:43

Akasurde self-requested a review March 5, 2019 05:34

Add unit tests

a5c8688

Only add tests for Clear Linux parsing since that was the cause of this issue.

bcoca removed the needs_triage Needs a first human triage before being processed. label Mar 5, 2019

samdoran added the P3 Priority 3 - Approved, No Time Limitation label Mar 6, 2019

ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 6, 2019

maxamillion approved these changes Mar 7, 2019

View reviewed changes

ansibot added shipit This PR is ready to be merged by Core and removed core_review In order to be merged, this PR must follow the core review workflow. labels Mar 7, 2019

Akasurde approved these changes Mar 8, 2019

View reviewed changes

samdoran merged commit 1d91e03 into ansible:devel Mar 8, 2019

samdoran deleted the clear-linux-coreos-distribution branch March 8, 2019 15:40

samdoran mentioned this pull request Mar 8, 2019

[stable-2.7] Ensure Clear Linux parsing is actually parsing a Clear Linux host and all others fall back to NA (#53298) #53541

Merged

ansible locked and limited conversation to collaborators Jul 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure Clear Linux parsing is actually parsing a Clear Linux host and all others fall back to NA #53298

Ensure Clear Linux parsing is actually parsing a Clear Linux host and all others fall back to NA #53298

samdoran commented Mar 4, 2019 •

edited

Akasurde commented Mar 5, 2019

maxamillion commented Mar 7, 2019

Akasurde commented Mar 8, 2019

Ensure Clear Linux parsing is actually parsing a Clear Linux host and all others fall back to NA #53298

Ensure Clear Linux parsing is actually parsing a Clear Linux host and all others fall back to NA #53298

Conversation

samdoran commented Mar 4, 2019 • edited

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

Akasurde commented Mar 5, 2019

maxamillion commented Mar 7, 2019

Akasurde commented Mar 8, 2019

samdoran commented Mar 4, 2019 •

edited