Skip to content
This repository has been archived by the owner on Jun 4, 2021. It is now read-only.

Add new option cacerts. #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

abergmeier
Copy link

This way you can override the CAs, that are used for http communication.

@@ -55,6 +64,11 @@ def main():
if not args.name or not args.directory:
raise Exception('--name and --directory are required arguments.')

if args.cacerts:
logging.info('Adding CA certificates of %s', args.cacerts)
retry_transport.DEFAULT_SOURCE_TRANSPORT_CALLABLE = _apply_ca_certs(
Copy link
Contributor

Choose a reason for hiding this comment

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

This hack shouldn't be necessary. More commentary in #51

Copy link
Author

Choose a reason for hiding this comment

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

Ok, just ping me when you rewrote the retry and I'll change this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

roger

Copy link
Contributor

Choose a reason for hiding this comment

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

@abergmeier I just upstreamed the internal change. If you rebase, you can supply the RetryTransport factory with your own source transport callable (i.e. a lambda that instantiates a httplib2.Http with the desired certs)

@abergmeier
Copy link
Author

Rebased.

def _apply_ca_certs(callable, ca_certs):
"""Apply ca_certs attribute to default transport"""
def _call_with_a_certs(*args, **kwargs):
return callable(*args, ca_certs = ca_certs, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

kwargs['ca_certs'] = ca_certs

@@ -49,6 +51,13 @@
_THREADS = 8


def _apply_ca_certs(callable, ca_certs):
"""Apply ca_certs attribute to default transport"""
def _call_with_a_certs(*args, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a_certs -> ca_certs

"""Apply ca_certs attribute to default transport"""
def _call_with_a_certs(*args, **kwargs):
return callable(*args, ca_certs = ca_certs, **kwargs)
return _call_with_a_certs
Copy link
Contributor

Choose a reason for hiding this comment

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

ca_certs

if args.cacerts:
logging.info('Adding CA certificates of %s', args.cacerts)
transport_callable = _apply_ca_certs(
transport_callable, args.cacerts)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd make this explicitly httplib2.Http, since none of our custom transports take that arg

This way you can override the CAs, that are used for http communication.
@abergmeier
Copy link
Author

Trying to add tests...

retry_factory = retry.Factory()
retry_factory = retry_factory.WithSourceTransportCallable(httplib2.Http)
retry_factory = retry_factory.WithSourceTransportCallable(transport_callable)
Copy link

@chrischdi chrischdi Jan 25, 2018

Choose a reason for hiding this comment

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

I think there should be an if clause?
otherwise transport_callable does not exist

if args.cacerts:
  retry_factory = retry_factory.WithSourceTransportCallable(transport_callable)
else:
  retry_factory = retry_factory.WithSourceTransportCallable(httplib2.Http)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants