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

add openssh_cert module #49605

Open
wants to merge 6 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@lolcube
Contributor

lolcube commented Dec 6, 2018

SUMMARY

Adds a new module for generating openssh user and host certificates

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

openssh_cert

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 6, 2018

@felixfontein

This comment has been minimized.

Contributor

felixfontein commented Dec 6, 2018

@lolcube thanks for your PR! There are some sanity tests which fail, please resolve these. Unfortunately, ansibot seems to no longer post this to the PR, but you can find the details here:

https://app.shippable.com/github/ansible/ansible/runs/96979/1/tests
https://app.shippable.com/github/ansible/ansible/runs/96979/2/tests
https://app.shippable.com/github/ansible/ansible/runs/96979/3/tests

needs_revision

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 6, 2018

The test ansible-test sanity --test pylint [explain] failed with 41 errors:

lib/ansible/modules/crypto/openssh_cert.py:233:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:246:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:248:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:251:31: bad-whitespace No space allowed after bracket                    validity += ( str(timeobj.year).zfill(4) +                                ^
lib/ansible/modules/crypto/openssh_cert.py:258:20: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:262:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:267:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:268:27: bad-whitespace No space allowed after bracket                validity += ( str(timeobj.year).zfill(4) +                            ^
lib/ansible/modules/crypto/openssh_cert.py:274:24: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:285:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:288:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:291:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:293:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:321:39: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:322:0: anomalous-backslash-in-string Anomalous backslash in string: '\-'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:322:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:322:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:322:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:322:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:322:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:328:56: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:329:55: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:330:56: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:335:56: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:336:55: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:337:56: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:366:0: anomalous-backslash-in-string Anomalous backslash in string: '\d'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/openssh_cert.py:423:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:425:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:481:35: bad-whitespace Exactly one space required after comma             path=dict(required=True,type='path'),                                    ^
lib/ansible/modules/crypto/openssh_cert.py:535:30: trailing-whitespace Trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:536:0: trailing-whitespace Trailing whitespace

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

lib/ansible/modules/crypto/openssh_cert.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/crypto/openssh_cert.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/crypto/openssh_cert.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/crypto/openssh_cert.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/crypto/openssh_cert.py:0:0: has a documentation error formatting or is missing documentation.

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
Command 'make singlehtmldocs' failed with status code: 2
--> Standard Output
cat _themes/srtd/static/css/theme.css | sed -e 's/^[ 	]*//g; s/[ 	]*$//g; s/\([:{;,]\) /\1/g; s/ {/{/g; s/\/\*.*\*\///g; /^$/d' | sed -e :a -e '$!N; s/\n\(.\)/\1/; ta' > _themes/srtd/static/css/theme.min.css
PYTHONPATH=../../lib ../bin/dump_config.py --template-file=../templates/config.rst.j2 --output-dir=rst/reference_appendices/ -d ../../lib/ansible/config/base.yml
mkdir -p rst/cli
PYTHONPATH=../../lib ../bin/generate_man.py --template-file=../templates/cli_rst.j2 --output-dir=rst/cli/ --output-format rst ../../lib/ansible/cli/*.py
PYTHONPATH=../../lib ../bin/dump_keywords.py --template-dir=../templates --output-dir=rst/reference_appendices/ -d ./keyword_desc.yml
PYTHONPATH=../../lib ../bin/plugin_formatter.py -t rst --template-dir=../templates --module-dir=../../lib/ansible/modules -o rst/modules/ 
Makefile:93: recipe for target 'modules' failed
--> Standard Error
Traceback (most recent call last):
  File "../bin/plugin_formatter.py", line 720, in <module>
    main()
  File "../bin/plugin_formatter.py", line 678, in main
    plugin_info, categories = get_plugin_info(options.module_dir, limit_to=options.limit_to, verbose=(options.verbosity > 0))
  File "../bin/plugin_formatter.py", line 269, in get_plugin_info
    doc, examples, returndocs, metadata = plugin_docs.get_docstring(module_path, fragment_loader, verbose=verbose)
  File "/root/ansible/lib/ansible/utils/plugin_docs.py", line 96, in get_docstring
    data = read_docstring(filename, verbose=verbose, ignore_errors=ignore_errors)
  File "/root/ansible/lib/ansible/parsing/plugin_docs.py", line 59, in read_docstring
    data[varkey] = AnsibleLoader(child.value.s, file_name=filename).get_single_data()
  File "/usr/local/lib/python3.6/dist-packages/yaml/constructor.py", line 35, in get_single_data
    node = self.get_single_node()
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 36, in get_single_node
    document = self.compose_document()
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 55, in compose_document
    node = self.compose_node(None, None)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 84, in compose_node
    node = self.compose_mapping_node(anchor)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 133, in compose_mapping_node
    item_value = self.compose_node(node, item_key)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 84, in compose_node
    node = self.compose_mapping_node(anchor)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 133, in compose_mapping_node
    item_value = self.compose_node(node, item_key)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 84, in compose_node
    node = self.compose_mapping_node(anchor)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 133, in compose_mapping_node
    item_value = self.compose_node(node, item_key)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 82, in compose_node
    node = self.compose_sequence_node(anchor)
  File "/usr/local/lib/python3.6/dist-packages/yaml/composer.py", line 110, in compose_sequence_node
    while not self.check_event(SequenceEndEvent):
  File "/usr/local/lib/python3.6/dist-packages/yaml/parser.py", line 98, in check_event
    self.current_event = self.state()
  File "/usr/local/lib/python3.6/dist-packages/yaml/parser.py", line 393, in parse_block_sequence_entry
    "expected <block end>, but found %r" % token.id, token.start_mark)
yaml.parser.ParserError: while parsing a block collection
  in "<unicode string>", line 44, column 13:
                - "The point in time the certifi ... 
                ^
expected <block end>, but found '<scalar>'
  in "<unicode string>", line 45, column 107:
     ... :MM:SS | YYYY:MM:DD HH:MM:SS | "always" where timespec can be an ... 
                                         ^
make: *** [modules] Error 1

The test ansible-test sanity --test pep8 [explain] failed with 93 errors:

lib/ansible/modules/crypto/openssh_cert.py:60:161: E501 line too long (175 > 160 characters)
lib/ansible/modules/crypto/openssh_cert.py:61:161: E501 line too long (187 > 160 characters)
lib/ansible/modules/crypto/openssh_cert.py:66:161: E501 line too long (173 > 160 characters)
lib/ansible/modules/crypto/openssh_cert.py:67:161: E501 line too long (188 > 160 characters)
lib/ansible/modules/crypto/openssh_cert.py:72:161: E501 line too long (166 > 160 characters)
lib/ansible/modules/crypto/openssh_cert.py:77:161: E501 line too long (165 > 160 characters)
lib/ansible/modules/crypto/openssh_cert.py:118:161: E501 line too long (195 > 160 characters)
lib/ansible/modules/crypto/openssh_cert.py:233:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:246:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:248:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:249:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:250:20: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:251:20: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:251:33: E201 whitespace after '('
lib/ansible/modules/crypto/openssh_cert.py:252:28: E128 continuation line under-indented for visual indent
lib/ansible/modules/crypto/openssh_cert.py:253:28: E126 continuation line over-indented for hanging indent
lib/ansible/modules/crypto/openssh_cert.py:257:24: E121 continuation line under-indented for hanging indent
lib/ansible/modules/crypto/openssh_cert.py:258:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:258:21: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:259:20: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:259:48: E261 at least two spaces before inline comment
lib/ansible/modules/crypto/openssh_cert.py:261:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:262:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:263:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:264:19: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:264:54: E261 at least two spaces before inline comment
lib/ansible/modules/crypto/openssh_cert.py:264:161: E501 line too long (161 > 160 characters)
lib/ansible/modules/crypto/openssh_cert.py:265:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:266:19: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:267:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:268:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:268:29: E201 whitespace after '('
lib/ansible/modules/crypto/openssh_cert.py:269:25: E128 continuation line under-indented for visual indent
lib/ansible/modules/crypto/openssh_cert.py:270:25: E126 continuation line over-indented for hanging indent
lib/ansible/modules/crypto/openssh_cert.py:274:24: E126 continuation line over-indented for hanging indent
lib/ansible/modules/crypto/openssh_cert.py:274:25: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:276:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:285:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:288:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:290:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:291:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:293:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:317:14: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:319:53: E261 at least two spaces before inline comment
lib/ansible/modules/crypto/openssh_cert.py:321:40: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:322:18: W605 invalid escape sequence '\-'
lib/ansible/modules/crypto/openssh_cert.py:322:24: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:322:35: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:322:46: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:322:57: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:322:68: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:328:57: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:329:56: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:330:57: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:335:57: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:336:56: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:337:57: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:350:5: E303 too many blank lines (2)
lib/ansible/modules/crypto/openssh_cert.py:366:21: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:366:27: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:366:33: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:366:39: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:366:46: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:366:61: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:366:67: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:366:73: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:366:79: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:366:86: W605 invalid escape sequence '\d'
lib/ansible/modules/crypto/openssh_cert.py:368:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:369:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:371:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:372:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:390:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:392:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:395:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:397:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:399:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:402:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:403:19: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:405:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:406:19: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:409:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:410:19: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:413:16: E111 indentation is not a multiple of four
lib/ansible/modules/crypto/openssh_cert.py:418:19: E271 multiple spaces after keyword
lib/ansible/modules/crypto/openssh_cert.py:420:15: E271 multiple spaces after keyword
lib/ansible/modules/crypto/openssh_cert.py:423:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:425:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:454:7: E122 continuation line missing indentation or outdented
lib/ansible/modules/crypto/openssh_cert.py:481:36: E231 missing whitespace after ','
lib/ansible/modules/crypto/openssh_cert.py:535:31: W291 trailing whitespace
lib/ansible/modules/crypto/openssh_cert.py:536:1: W293 blank line contains whitespace
lib/ansible/modules/crypto/openssh_cert.py:537:1: E305 expected 2 blank lines after class or function definition, found 1

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

lib/ansible/modules/crypto/openssh_cert.py:0:0: E319 RETURN.info.returned: required key not provided @ data['info']['returned']. Got None
lib/ansible/modules/crypto/openssh_cert.py:0:0: E319 RETURN.info.type: required key not provided @ data['info']['type']. Got None
lib/ansible/modules/crypto/openssh_cert.py:0:0: E324 Value for "default" from the argument_spec ('present') for "state" does not match the documentation (None)
lib/ansible/modules/crypto/openssh_cert.py:0:0: E324 Value for "default" from the argument_spec (False) for "force" does not match the documentation (None)
lib/ansible/modules/crypto/openssh_cert.py:0:0: E326 Value for "choices" from the argument_spec (['host', 'user']) for "type" does not match the documentation ([])
lib/ansible/modules/crypto/openssh_cert.py:0:0: E326 Value for "choices" from the argument_spec (['present', 'absent']) for "state" does not match the documentation ([])
lib/ansible/modules/crypto/openssh_cert.py:61:107: E302 DOCUMENTATION is not valid YAML

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

lib/ansible/modules/crypto/openssh_cert.py:61:107: error DOCUMENTATION: syntax error: expected <block end>, but found '<scalar>'

click here for bot help

@felixfontein

This comment has been minimized.

Contributor

felixfontein commented Dec 6, 2018

Ah, here we go.

@MarkusTeufelberger

This comment has been minimized.

Contributor

MarkusTeufelberger commented Dec 6, 2018

Poor little Ansibot probably just was overwhelmed a bit. :-P

lolcube added some commits Dec 8, 2018

@felixfontein

This comment has been minimized.

Contributor

felixfontein commented Dec 9, 2018

!needs_revision

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 9, 2018

@MarkusTeufelberger @gdelpierre @john-westcott-iv @mgruener @thomwiggers

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

path:
required: true
description:
- Path of the file containing the certificate.

This comment has been minimized.

@felixfontein

felixfontein Dec 9, 2018

Contributor

You should add type: path here.

This comment has been minimized.

@felixfontein

felixfontein Dec 10, 2018

Contributor

You could of course also add it to the other path options :-)

- "Certificates may be limited to be valid for a set of principal (user/host) names.
By default, generated certificates are valid for all users or hosts."
options:
required: false

This comment has been minimized.

@felixfontein

felixfontein Dec 9, 2018

Contributor

type: list is missing.

description:
- "Specify a certificate options when signing a key. The option that are valid for user certificates are:
clear Clear all enabled permissions. This is useful for clearing the default set of permissions so permissions may be added individually.

This comment has been minimized.

@felixfontein

felixfontein Dec 9, 2018

Contributor

This will probably not be formatted as you intend it.

How about:

- "Specify a certificate options when signing a key. The option that are valid for user certificates are:"
- "C(clear): Clear all enabled permissions.  This is useful for clearing the default set of permissions so permissions may be added individually."
- "C(force-command=command): Forces the execution of command instead of any shell or command specified by the user when the certificate is used for authentication."
- "C(no-agent-forwarding): Disable ssh-agent forwarding (permitted by default)."
...
options:
required: false
description:
- "Specify a certificate options when signing a key. The option that are valid for user certificates are:

This comment has been minimized.

@felixfontein

felixfontein Dec 9, 2018

Contributor
Suggested change Beta
- "Specify a certificate options when signing a key. The option that are valid for user certificates are:
- "Specify certificate options when signing a key. The options that are valid for user certificates are:
path: /path/to/certificate
valid_from: +0s
valid_to: +32w
valid_at: +2w

This comment has been minimized.

@felixfontein

felixfontein Dec 9, 2018

Contributor

You should also add an example with options:.

type: str
sample: /tmp/certifivate-cert.pub
info:
description: Information about the certificate. Output of "ssh-keygen -L -f".

This comment has been minimized.

@felixfontein

felixfontein Dec 9, 2018

Contributor
Suggested change Beta
description: Information about the certificate. Output of "ssh-keygen -L -f".
description: Information about the certificate. Output of C(ssh-keygen -L -f).
if not self.isValid(module, perms_required=False) or self.force:
args = [
module.get_bin_path('ssh-keygen', True),

This comment has been minimized.

@felixfontein

felixfontein Dec 9, 2018

Contributor

Why don't you compute this once (during __init__()) and store it in an attribute?

This comment has been minimized.

@felixfontein

felixfontein Dec 10, 2018

Contributor

I didn't mean self.isValid() (that doesn't really make sense), but module.get_bin_path('ssh-keygen', True).

This comment has been minimized.

@lolcube

lolcube Dec 11, 2018

Contributor

Well, sorry that was quite stupid

if module.check_mode:
result = certificate.dump()
result['changed'] = module.params['force'] or not certificate.isValid(module)

This comment has been minimized.

@felixfontein

felixfontein Dec 9, 2018

Contributor

Why not set certificate.changed here as well (as for state == 'absent'), and then put the non-check part into an else: block (similarly for the absent part), and use the same result = certificate.dump() / module.exit_json(**result) for all code paths?

This comment has been minimized.

@lolcube

lolcube Dec 10, 2018

Contributor

you're right i will fix that

lolcube added some commits Dec 10, 2018

string = ""
for word in arr:
if word in keywords:
concated.extend([string])

This comment has been minimized.

@felixfontein

felixfontein Dec 12, 2018

Contributor

Why don't you do concated.append(string)? That's more efficient and easier to read.

return False
if self.valid_at:
if cert_valid_from <= self.convertToDatetime(module, self.vaid_at) <= cert_valid_to:

This comment has been minimized.

@felixfontein

felixfontein Dec 12, 2018

Contributor
Suggested change Beta
if cert_valid_from <= self.convertToDatetime(module, self.vaid_at) <= cert_valid_to:
if cert_valid_from <= self.convertToDatetime(module, self.valid_at) <= cert_valid_to:
if _check_state():
proc = module.run_command([self.ssh_keygen, '-L', '-f', self.path], environ_update=dict(TZ="UTC"))
self.cert_info = proc[1].split()
principals = re.findall("(?<=Principals:)(.*)(?=Critical)", proc[1], re.S)[0].split()

This comment has been minimized.

@felixfontein

felixfontein Dec 12, 2018

Contributor

What will happen if ssh-keygen outputs something different (like not a valid file)? This will probably break.

This comment has been minimized.

@felixfontein

felixfontein Dec 12, 2018

Contributor

(In fact, run_command() might already die when ssh-keygen returns a non-zero return code. I think there's a flag for run_command() to force it to not to do that.)

This comment has been minimized.

@lolcube

lolcube Dec 12, 2018

Contributor

:kw check_rc: Whether to call fail_json in case of non zero RC.
Default False

This comment has been minimized.

@felixfontein

felixfontein Dec 14, 2018

Contributor

I would add check_rc=False anyway, to make clearer your intention that you accept non-zero return codes.

def convertToDatetime(self, module, timestring):
if self.isRelative(timestring):
dispatched_time = re.findall("^([+\\-])((\\d+)[w])?((\\d+)[d])?((\\d+)[h])?((\\d+)[m])?((\\d+)[s])?$", timestring, re.I)

This comment has been minimized.

@felixfontein

felixfontein Dec 12, 2018

Contributor

Why do you use findall, when this can match only once anyway?

This comment has been minimized.

@lolcube

lolcube Dec 13, 2018

Contributor

Because findall is way easier to work with and returns a nice list containing everything i need.

if module.set_fs_attributes_if_different(file_args, False):
self.changed = True
def convertToDatetime(self, module, timestring):

This comment has been minimized.

@felixfontein

felixfontein Dec 12, 2018

Contributor

Out of curiosity: is there a reason why you use camelCase instead of under_scores for method names?

This comment has been minimized.

@lolcube

lolcube Dec 12, 2018

Contributor

Because I am used to it and I personally think it is easier to read. However i know that it's much more common to use snake case in python and I forgot to rename that one.

This comment has been minimized.

@felixfontein

felixfontein Dec 14, 2018

Contributor

I tried to use camelCase in Python for some time some years ago for the same reason, but eventually gave up (since all library calls etc. don't use it, and mixing both styles is even worse ;) ) and switched to snake_case. I was just curious what's your motivation ;)

type: str
description:
- "Check if the certificate is valid at a certain point in time. If it is not the certificate will be regenerated.
Time will always be interpreted as UTC. Mainly to be used with relative timespec for valid_from and / or valid_to.

This comment has been minimized.

@thomwiggers

thomwiggers Dec 12, 2018

Contributor

I think it'd be better readable if you do C(valid_from) and C(valid_to)

- "C(no-agent-forwarding): Disable ssh-agent forwarding (permitted by default)."
- "C(no-port-forwarding): Disable port forwarding (permitted by default)."
- "C(no-pty Disable): PTY allocation (permitted by default)."
- "C(no-user-rc): Disable execution of ~/.ssh/rc by sshd (permitted by default)."

This comment has been minimized.

@thomwiggers

thomwiggers Dec 12, 2018

Contributor
Suggested change Beta
- "C(no-user-rc): Disable execution of ~/.ssh/rc by sshd (permitted by default)."
- "C(no-user-rc): Disable execution of C(~/.ssh/rc) by sshd (permitted by default)."
- "C(permit-agent-forwarding): Allows ssh-agent forwarding."
- "C(permit-port-forwarding): Allows port forwarding."
- "C(permit-pty): Allows PTY allocation."
- "C(permit-user-rc): Allows execution of ~/.ssh/rc by sshd."

This comment has been minimized.

@thomwiggers

thomwiggers Dec 12, 2018

Contributor
Suggested change Beta
- "C(permit-user-rc): Allows execution of ~/.ssh/rc by sshd."
- "C(permit-user-rc): Allows execution of C(~/.ssh/rc) by sshd."
type: str
description:
- "Check if the certificate is valid at a certain point in time. If it is not the certificate will be regenerated.
Time will always be interpreted as UTC. Mainly to be used with relative timespec for valid_from and / or valid_to.

This comment has been minimized.

@thomwiggers

thomwiggers Dec 12, 2018

Contributor
Suggested change Beta
Time will always be interpreted as UTC. Mainly to be used with relative timespec for valid_from and / or valid_to.
Time will always be interpreted as UTC. Mainly to be used with relative timespec for C(valid_from) and / or C(valid_to).

This comment has been minimized.

@felixfontein

felixfontein Dec 13, 2018

Contributor

Shouldn't it be I(valid_from) / I(valid_to), since this is about option names?

type: str
description:
- "The point in time the certificate is valid to. Time can be specified either as relative time or as absolute timestamp.
Time will always be interpreted as UTC. Valid formats are: [+-]timespec | YYYY:MM:DD | YYYY:MM:DD:HH:MM:SS | YYYY:MM:DD HH:MM:SS | forever

This comment has been minimized.

@thomwiggers

thomwiggers Dec 12, 2018

Contributor

I think C() should help with formatting and readability

Suggested change Beta
Time will always be interpreted as UTC. Valid formats are: [+-]timespec | YYYY:MM:DD | YYYY:MM:DD:HH:MM:SS | YYYY:MM:DD HH:MM:SS | forever
Time will always be interpreted as UTC. Valid formats are: C([+-]timespec | YYYY:MM:DD | YYYY:MM:DD:HH:MM:SS | YYYY:MM:DD HH:MM:SS | forever)
description:
- "The point in time the certificate is valid to. Time can be specified either as relative time or as absolute timestamp.
Time will always be interpreted as UTC. Valid formats are: [+-]timespec | YYYY:MM:DD | YYYY:MM:DD:HH:MM:SS | YYYY:MM:DD HH:MM:SS | forever
where timespec can be an integer + [w | d | h | m | s] (e.g. +32w1d2h).

This comment has been minimized.

@thomwiggers

thomwiggers Dec 12, 2018

Contributor
Suggested change Beta
where timespec can be an integer + [w | d | h | m | s] (e.g. +32w1d2h).
where timespec can be an integer C(+ [w | d | h | m | s]) (e.g. C(+32w1d2h)).
- "C(permit-user-rc): Allows execution of ~/.ssh/rc by sshd."
- "C(permit-x11-forwarding): Allows X11 forwarding."
- "C(source-address=address_list): Restrict the source addresses from which the certificate is considered valid.
The address_list is a comma-separated list of one or more address/netmask pairs in CIDR format."

This comment has been minimized.

@thomwiggers

thomwiggers Dec 12, 2018

Contributor
Suggested change Beta
The address_list is a comma-separated list of one or more address/netmask pairs in CIDR format."
The C(address_list) is a comma-separated list of one or more address/netmask pairs in CIDR format."

This comment has been minimized.

@felixfontein

felixfontein Dec 14, 2018

Contributor

@thomwiggers you also seem to like the "suggested change" feature :D

type: str
description:
- "Check if the certificate is valid at a certain point in time. If it is not the certificate will be regenerated.
Time will always be interpreted as UTC. Mainly to be used with relative timespec for C(valid_from) and / or C(valid_to).

This comment has been minimized.

@felixfontein

felixfontein Dec 14, 2018

Contributor

Ansible's developer documentation says I(...) should be used for options.

Suggested change Beta
Time will always be interpreted as UTC. Mainly to be used with relative timespec for C(valid_from) and / or C(valid_to).
Time will always be interpreted as UTC. Mainly to be used with relative timespec for I(valid_from) and / or I(valid_to).
if _check_state():
proc = module.run_command([self.ssh_keygen, '-L', '-f', self.path], environ_update=dict(TZ="UTC"))
self.cert_info = proc[1].split()
principals = re.findall("(?<=Principals:)(.*)(?=Critical)", proc[1], re.S)[0].split()

This comment has been minimized.

@felixfontein

felixfontein Dec 14, 2018

Contributor

I would add check_rc=False anyway, to make clearer your intention that you accept non-zero return codes.

@felixfontein

This comment has been minimized.

Contributor

felixfontein commented Dec 15, 2018

A test playbook I'm using to test the module (could be the basis for an integration test):

---
- hosts: localhost
  tasks:
  - name: Generate keypair (check mode)
    openssh_keypair:
      path: id_key
      type: ed25519
    check_mode: yes
  - name: Generate keypair
    openssh_keypair:
      path: id_key
      type: ed25519
  - name: Generate keypair (idempotent)
    openssh_keypair:
      path: id_key
      type: ed25519
  - name: Generate keypair (idempotent, check mode)
    openssh_keypair:
      path: id_key
      type: ed25519
    check_mode: yes
  - name: Generate always valid cert (check mode)
    openssh_cert:
      type: user
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: always
      valid_to: forever
    check_mode: yes
  - name: Generate always valid cert
    openssh_cert:
      type: user
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: always
      valid_to: forever
  - name: Generate always valid cert (idempotent)
    openssh_cert:
      type: user
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: always
      valid_to: forever
  - name: Generate always valid cert (idempotent, check mode)
    openssh_cert:
      type: user
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: always
      valid_to: forever
    check_mode: yes
  - name: Generate restricted validity cert with valid_at (check mode)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: +0s
      valid_to: +32w
      valid_at: +2w
    check_mode: yes
  - name: Generate restricted validity cert with valid_at
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: +0s
      valid_to: +32w
      valid_at: +2w
  - name: Generate restricted validity cert with valid_at (idempotent)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: +0s
      valid_to: +32w
      valid_at: +2w
  - name: Generate restricted validity cert with valid_at (idempotent, check mode)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: +0s
      valid_to: +32w
      valid_at: +2w
    check_mode: yes
  - name: Generate always valid cert only for example.com and examplehost (check mode)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: always
      valid_to: forever
      principals:
          - example.com
          - examplehost
    check_mode: yes
  - name: Generate always valid cert only for example.com and examplehost
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: always
      valid_to: forever
      principals:
          - example.com
          - examplehost
  - name: Generate always valid cert only for example.com and examplehost (idempotent)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: always
      valid_to: forever
      principals:
          - example.com
          - examplehost
  - name: Generate always valid cert only for example.com and examplehost (idempotent, check mode)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: always
      valid_to: forever
      principals:
          - example.com
          - examplehost
    check_mode: yes
  - name: Generate always valid cert only for example.com and examplehost (idempotent, switch)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: always
      valid_to: forever
      principals:
          - examplehost
          - example.com
  - name: Generate OpenSSH host Certificate that is valid from 21.1.2001 to 21.1.2019 (check mode)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: "2001:01:21"
      valid_to: "2019:01:21"
    check_mode: yes
  - name: Generate OpenSSH host Certificate that is valid from 21.1.2001 to 21.1.2019
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: "2001:01:21"
      valid_to: "2019:01:21"
  - name: Generate OpenSSH host Certificate that is valid from 21.1.2001 to 21.1.2019 (idempotent)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: "2001:01:21"
      valid_to: "2019:01:21"
  - name: Generate OpenSSH host Certificate that is valid from 21.1.2001 to 21.1.2019 (idempotent, check mode)
    openssh_cert:
      type: host
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      valid_from: "2001:01:21"
      valid_to: "2019:01:21"
    check_mode: yes
  - name: Generate an OpenSSH user Certificate with clear and force-command option (check mode)
    openssh_cert:
      type: user
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      options:
          - "clear"
          - "force-command=/tmp/bla/foo"
      valid_from: "2001:01:21"
      valid_to: "2019:01:21"
    check_mode: yes
  - name: Generate an OpenSSH user Certificate with clear and force-command option
    openssh_cert:
      type: user
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      options:
          - "clear"
          - "force-command=/tmp/bla/foo"
      valid_from: "2001:01:21"
      valid_to: "2019:01:21"
  - name: Generate an OpenSSH user Certificate with clear and force-command option (idempotent)
    openssh_cert:
      type: user
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      options:
          - "clear"
          - "force-command=/tmp/bla/foo"
      valid_from: "2001:01:21"
      valid_to: "2019:01:21"
  - name: Generate an OpenSSH user Certificate with clear and force-command option (idempotent, check mode)
    openssh_cert:
      type: user
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      options:
          - "clear"
          - "force-command=/tmp/bla/foo"
      valid_from: "2001:01:21"
      valid_to: "2019:01:21"
    check_mode: yes
  - name: Generate an OpenSSH user Certificate with clear and force-command option (idempotent, switch)
    openssh_cert:
      type: user
      signing_key: id_key
      public_key: id_key.pub
      path: id_cert
      options:
          - "force-command=/tmp/bla/foo"
          - "clear"
      valid_from: "2001:01:21"
      valid_to: "2019:01:21"
  - name: Remove certificate (check mode)
    openssh_cert:
      state: absent
      path: id_cert
      #type: user
      #signing_key: id_key
      #public_key: id_key.pub
      #valid_from: "2001:01:21"
      #valid_to: "2019:01:21"
    check_mode: yes
  - name: Remove certificate
    openssh_cert:
      state: absent
      path: id_cert
      #type: user
      #signing_key: id_key
      #public_key: id_key.pub
      #valid_from: "2001:01:21"
      #valid_to: "2019:01:21"
  - name: Remove certificate (idempotent)
    openssh_cert:
      state: absent
      path: id_cert
      #type: user
      #signing_key: id_key
      #public_key: id_key.pub
      #valid_from: "2001:01:21"
      #valid_to: "2019:01:21"
  - name: Remove certificate (idempotent, check mode)
    openssh_cert:
      state: absent
      path: id_cert
      #type: user
      #signing_key: id_key
      #public_key: id_key.pub
      #valid_from: "2001:01:21"
      #valid_to: "2019:01:21"
    check_mode: yes
  - name: Remove keypair (check mode)
    openssh_keypair:
      path: id_key
      state: absent
    check_mode: yes
  - name: Remove keypair
    openssh_keypair:
      path: id_key
      state: absent
  - name: Remove keypair (idempotent)
    openssh_keypair:
      path: id_key
      state: absent
  - name: Remove keypair (idempotent, check mode)
    openssh_keypair:
      path: id_key
      state: absent
    check_mode: yes
public_key: /path/to/public_key.pub
path: /path/to/certificate
valid_from: 2001:01:21
valid_to: 2019:01:21

This comment has been minimized.

@felixfontein

felixfontein Dec 15, 2018

Contributor

This isn't valid YAML. You have to quote it:

    valid_from: "2001:01:21"
    valid_to: "2019:01:21"

Also, using colons as date separators seems very strange for me. I would expect slashes or dots.

valid_to: 2019:01:21
# Generate an OpenSSH user Certificate with clear and force-command option:
- openssh-cert:

This comment has been minimized.

@felixfontein

felixfontein Dec 15, 2018

Contributor
Suggested change Beta
- openssh-cert:
- openssh_cert:
)
def isBaseDir(path):
base_dir = os.path.dirname(path)

This comment has been minimized.

@felixfontein

felixfontein Dec 15, 2018

Contributor
Suggested change Beta
base_dir = os.path.dirname(path)
base_dir = os.path.dirname(path) or '.'

Otherwise, path == 'file' (i.e. file in current directory) will result in fatal: [localhost]: FAILED! => {"changed": false, "msg": "The directory does not exist or the file is not a directory", "name": ""}

The same thing goes wrong in openssh_keypair, BTW.

path: /path/to/certificate
options:
- "clear"
- "force-command=/tmp/bla/foo"

This comment has been minimized.

@felixfontein

felixfontein Dec 15, 2018

Contributor

This task fails with {"changed": false, "msg": "missing required arguments: valid_from, valid_to"}

module = AnsibleModule(
argument_spec=dict(
state=dict(default='present', choices=['present', 'absent'], type='str'),

This comment has been minimized.

@felixfontein

felixfontein Dec 15, 2018

Contributor

state=absent requires a lot of additional arguments which are not needed: {"changed": false, "msg": "missing required arguments: type, signing_key, public_key, valid_from, valid_to"}

These arguments should only be required when state is 'present'. Look at the required_if parameter of AnsibleModule().

return self.type == cert_type
def _check_principals():
return self.principals == principals

This comment has been minimized.

@felixfontein

felixfontein Dec 15, 2018

Contributor

This check always fails on my machine: self.principals == None, principals == ['(none)']

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment