Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions jenkins_cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def main():
parser.add_argument('--host', metavar='jenkins-url', help='Jenkins Host', default=None)
parser.add_argument('--username', metavar='username', help='Jenkins Username', default=None)
parser.add_argument('--password', metavar='password', help='Jenkins Password', default=None)
parser.add_argument('--ignore-ssl', action='store_true', help='Disable SSL verification')
parser.add_argument('--version', '-v', action='version', version='jenkins-cli %s' % version)

subparsers = parser.add_subparsers(title='Available commands', dest='jenkins_command')
Expand Down
26 changes: 24 additions & 2 deletions jenkins_cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import sys
import datetime
from time import time, sleep
import ssl
import jenkins
import socket
from xml.etree import ElementTree
Expand Down Expand Up @@ -31,6 +32,10 @@
}


# Default constants to parse booleans from .jenkins-cli
TRUE = ['yes', 'Yes', 'YES', 'True', 'true', 'TRUE', '1']
Copy link
Owner

Choose a reason for hiding this comment

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

I think if ignore_ssl= option is in .jencins-cli file, that means that we do want to ignore ssl errors. So I think we do not need TRUE constant as well.

Copy link
Author

Choose a reason for hiding this comment

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

Well, I think it will be very weird if someone configures this property like this: ignore_ssl=False and the application continue ignoring SSL errors.

That's why I thought it would be a good idea to configure a list with all acceptable strings.

ignore_ssl= also seems strange to me. What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

I agree that ignore_ssl=False may looks a bit strange. But the list with all acceptable strings looks strange also.
Maybe it will be enough to define behavior in help section?

Something like:
"You can set ignore_ssl in settings file. Any value will be treated as True unless it equals 0"
or
"You can set ignore_ssl=1 in settings file if you want to ignore ssl erros. All other values will be treated as False"

We need to document this option anyway and its hardy possible to find all acceptable strings (for example we can also add 'y' and 'Y')

What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, any case scenario this should be documented on both README.md and --help.

We could document that the user should set ignore_ssl=yes and all other cases will be treated as False and still have the list.

If we decide to accept only one option the comparison will be pretty much the same:

List:

cls.ignore_ssl = ignore_ssl or settings_dict.get('ignore_ssl', 'False') in TRUE

Single accepted pattern:

cls.ignore_ssl = ignore_ssl or settings_dict.get('ignore_ssl', 'False') == '1'

BTW, I was inspired by ansible when I wrote this list. They pretty much have the same problem, reading config and transforming what it seems to be boolean into python bool.



ENDCOLLOR = '\033[0m'
ANIME_SYMBOL = ['..', '>>']
AUTHOR_COLLOR = '\033[94m'
Expand All @@ -44,6 +49,20 @@
}


def setup_ssl_context(ignore_ssl):
"""Disable SSL verification if ignore_ssl=True"""

if ignore_ssl:
try:
_create_unverified_https_context = ssl._create_unverified_context
except AttributeError:
# Legacy Python that doesn't verify HTTPS certificates by default
pass
else:
# Handle target environment that doesn't support HTTPS verification
ssl._create_default_https_context = _create_unverified_https_context


def get_formated_status(job_color, format_pattern="%(color)s%(symbol)s%(run_status)s%(endcollor)s", extra_params=None):
if not extra_params:
extra_params = {}
Expand Down Expand Up @@ -87,16 +106,18 @@ class JenkinsCli(object):
"%s branch set to: %s")

def __init__(self, args, timeout=socket._GLOBAL_DEFAULT_TIMEOUT):
self.jenkins = self.auth(args.host, args.username, args.password, timeout)
self.jenkins = self.auth(args.host, args.username, args.password, args.ignore_ssl, timeout)

@classmethod
def auth(cls, host=None, username=None, password=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT):
def auth(cls, host=None, username=None, password=None, ignore_ssl=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT):
if host is None or username is None or password is None:
settings_dict = cls.read_settings_from_file()
try:
host = host or settings_dict['host']
username = username or settings_dict.get('username', None)
password = password or settings_dict.get('password', None)
cls.ignore_ssl = ignore_ssl or settings_dict.get('ignore_ssl', 'False') in TRUE

except KeyError:
raise CliException('Jenkins "host" should be specified by the command-line option or in the .jenkins-cli file')
return jenkins.Jenkins(host, username, password, timeout)
Expand Down Expand Up @@ -124,6 +145,7 @@ def read_settings_from_file(cls):
return settings_dict

def run_command(self, args):
setup_ssl_context(self.ignore_ssl)
command = args.jenkins_command
getattr(self, command)(args)

Expand Down
1 change: 1 addition & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,5 +311,6 @@ def info_side_effect(name, number):
self.patched_print.assert_has_calls([mock.call("FDN Job1 estimated time left %s" % timedelta(seconds=TS))],
[mock.call("FDN Job5 estimated time left %s" % timedelta(seconds=TS))])


if __name__ == '__main__':
unittest.main()