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

Strip trailing comments from /etc/default/passwd #43931

Merged
merged 5 commits into from
Aug 15, 2018
Merged

Strip trailing comments from /etc/default/passwd #43931

merged 5 commits into from
Aug 15, 2018

Conversation

tomtastic
Copy link
Contributor

SUMMARY

Strip trailling comments from /etc/default/passwd like:
MINWEEKS=1 #MINWEEKS=2
MAXWEEKS=12 # MAXWEEKS=8
Which otherwise cause failures with "failed to read /etc/default/passwd: too many values to unpack"

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

user module

ANSIBLE VERSION
ansible 2.4.2.0
  config file = /development/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 = /usr/bin/ansible
  python version = 2.7.5 (default, May 31 2018, 09:41:32) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]

But present in the current 2.6+ releases too.

ADDITIONAL INFORMATION

Example input file

#ident  "@(#)passwd.dfl 1.8     14/01/24 SMI"
#
# Copyright (c) 1989, 2014, Oracle and/or its affiliates. All rights reserved.
#
MAXWEEKS=12 #MAXWEEKS=8
MINWEEKS=0  # MINWEEKS=4
PASSLENGTH=8
MINALPHA=1 # MINALPHA = 2
#WHITESPACE=YES
#

Old code fails with ValueError: too many values to unpack

import sys

def get_password_defaults():
    # Read password aging defaults
    minweeks = ''
    maxweeks = ''
    warnweeks = ''
    for line in open("/tmp/default_passwd", 'r'):
        line = line.strip()
        if (line.startswith('#') or line == ''):
            continue
        key, value = line.split('=')

        if key == "MINWEEKS":
            minweeks = value.rstrip('\n')
        elif key == "MAXWEEKS":
            maxweeks = value.rstrip('\n')
        elif key == "WARNWEEKS":
            warnweeks = value.rstrip('\n')

    return (minweeks, maxweeks, warnweeks)

minweeks, maxweeks, warnweeks = get_password_defaults()

Fixed code strips trailing comments

import sys
import re

def get_password_defaults():
    # Read password aging defaults
    minweeks = ''
    maxweeks = ''
    warnweeks = ''
    for line in open("/tmp/default_passwd", 'r'):
        line = line.strip()
        if (line.startswith('#') or line == ''):
            continue
        m = re.match(r'^([^#]*)#(.*)$', line)
        if m:  # The line contains a hash / comment
            line = m.group(1)
        key, value = line.split('=')

        if key == "MINWEEKS":
            minweeks = value.rstrip('\n')
        elif key == "MAXWEEKS":
            maxweeks = value.rstrip('\n')
        elif key == "WARNWEEKS":
            warnweeks = value.rstrip('\n')

    return (minweeks, maxweeks, warnweeks)

minweeks, maxweeks, warnweeks = get_password_defaults()

Tom Matthews added 3 commits August 10, 2018 12:04
Strip trailling comments from /etc/default/passwd like
MINWEEKS=1 #MINWEEKS=2
MAXWEEKS=12  # MAXWEEKS=8
Which otherwise cause failures with "failed to read /etc/default/passwd: too many values to unpack"
@ansibot
Copy link
Contributor

ansibot commented Aug 10, 2018

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Aug 10, 2018
@ansibot
Copy link
Contributor

ansibot commented Aug 10, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/system/user.py:1496:0: syntax-error expected an indented block (<string>, line 1496)

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:1496:20: SyntaxError: line = m.group(1)

The test ansible-test sanity --test compile --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:1496:20: SyntaxError: line = m.group(1)

The test ansible-test sanity --test compile --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:1496:20: SyntaxError: line = m.group(1)

The test ansible-test sanity --test compile --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:1496:20: SyntaxError: line = m.group(1)

The test ansible-test sanity --test compile --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:1496:20: SyntaxError: line = m.group(1)

The test ansible-test sanity --test docs-build [explain] failed with the error:

Command "/usr/bin/python test/sanity/code-smell/docs-build.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
  File "test/sanity/code-smell/docs-build.py", line 100, in <module>
    main()
  File "test/sanity/code-smell/docs-build.py", line 17, in main
    raise subprocess.CalledProcessError(sphinx.returncode, cmd, output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['make', 'singlehtmldocs']' returned non-zero exit status 2.

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:1496:20: IndentationError: expected an indented block

The test ansible-test sanity --test import --python 2.7 [explain] failed with 2 errors:

lib/ansible/modules/system/user.py:0:0: IndentationError: expected an indented block (user.py, line 1496) (in /root/ansible/test/runner/.tox/minimal-py27/bin/importer.py:82)
lib/ansible/modules/system/user.py:1496:20: IndentationError: expected an indented block

The test ansible-test sanity --test import --python 3.5 [explain] failed with 2 errors:

lib/ansible/modules/system/user.py:0:0: IndentationError: expected an indented block (user.py, line 1496) (in /root/ansible/test/runner/.tox/minimal-py35/bin/importer.py:82)
lib/ansible/modules/system/user.py:1496:20: IndentationError: expected an indented block

The test ansible-test sanity --test import --python 3.6 [explain] failed with 2 errors:

lib/ansible/modules/system/user.py:0:0: IndentationError: expected an indented block (user.py, line 1496) (in /root/ansible/test/runner/.tox/minimal-py36/bin/importer.py:82)
lib/ansible/modules/system/user.py:1496:20: IndentationError: expected an indented block

The test ansible-test sanity --test import --python 3.7 [explain] failed with 2 errors:

lib/ansible/modules/system/user.py:0:0: IndentationError: expected an indented block (user.py, line 1496) (in /root/ansible/test/runner/.tox/minimal-py37/bin/importer.py:82)
lib/ansible/modules/system/user.py:1496:20: IndentationError: expected an indented block

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/system/user.py:1496:17: E112 expected an indented block

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/system/user.py:0:0: E401 Python SyntaxError while parsing module
test/sanity/validate-modules/ignore.txt:1186:1: A102 Remove since "lib/ansible/modules/system/user.py" passes "E324" test
test/sanity/validate-modules/ignore.txt:1187:1: A102 Remove since "lib/ansible/modules/system/user.py" passes "E327" test

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

lib/ansible/modules/system/user.py:1496:20: python-syntax-error expected an indented block (<unknown>, line 1496)

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Aug 10, 2018
@ansibot 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 Aug 10, 2018
@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Aug 13, 2018
@samdoran samdoran self-assigned this Aug 14, 2018
@samdoran
Copy link
Contributor

I have validated this failure and that your fix works. Please create a changelog fragment and I'll merge this. See fragments for examples.

@@ -0,0 +1,3 @@
---
bugfixes:
- VMware Strip trailing comments in /etc/default/passwd (https://github.com/ansible/ansible/pull/43931)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the "VMware" string here. Start the summary with - user - which will help group the changelog entries by module they affect. Here is an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gah, copy paste error.

@ansibot ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Aug 14, 2018
@samdoran samdoran merged commit 5c1e620 into ansible:devel Aug 15, 2018
@samdoran
Copy link
Contributor

Please create a backport PR for this to be included in previous versions.

@tomtastic
Copy link
Contributor Author

@tomtastic tomtastic deleted the user_module_strip_comments branch August 19, 2018 15:47
@samdoran
Copy link
Contributor

@tomtastic Can you please backport to stable-2.6 as well?

@tomtastic
Copy link
Contributor Author

Yes of course, I forgot about that one

@samdoran
Copy link
Contributor

Thank you!

mattclay pushed a commit that referenced this pull request Sep 4, 2018
* strip additional comments from /etc/default/passwd

Strip trailling comments from /etc/default/passwd like
MINWEEKS=1 #MINWEEKS=2
MAXWEEKS=12  # MAXWEEKS=8
Which otherwise cause failures with "failed to read /etc/default/passwd: too many values to unpack"

* fix carriage return typo in commit

* yet another typo in commit

* Fix indent problem

* add changelog fragment for PR 43931

(cherry picked from commit 5c1e620)
nitzmahone pushed a commit that referenced this pull request Sep 5, 2018
* strip additional comments from /etc/default/passwd

Strip trailling comments from /etc/default/passwd like
MINWEEKS=1 #MINWEEKS=2
MAXWEEKS=12  # MAXWEEKS=8
Which otherwise cause failures with "failed to read /etc/default/passwd: too many values to unpack"

* fix carriage return typo in commit

* yet another typo in commit

* Fix indent problem

* add changelog fragment for PR 43931

(cherry picked from commit 5c1e620)
@gundalow
Copy link
Contributor

Hi,
I'm randomly selecting some PRs to see how it was for you, with the aim of making it easier for future contributors. As a new contributor I'm particularly interested as you don't have any prior knowledge of the Ansible process.

  1. Generally speaking, how was the process for you?
  2. Did it make sense what to do to progress the PR?
  3. Did you read any of the developer (dev_guide) or contributor documentation?
  4. What in the documentation was useful or confusing, missing or could be improved
  5. Did you know you could run ansible-test locally?
  6. What areas would you be interested in contributing to?

ansible/community#353

@gundalow gundalow added the contrib_follow_up https://github.com/ansible/community/issues/353 label Oct 27, 2018
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. contrib_follow_up https://github.com/ansible/community/issues/353 core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants