juju overrides http.DefaultTransport in general and specifically for tests

Bug #1888888 reported by Heather Lanigan
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Canonical Juju
Triaged
Low
Unassigned

Bug Description

Ideally http.NewClient should return a constructed Transport so that we can make suitable changes, such as not caching the proxy settings and creating a TLS config. Due to the way tests are setup in juju/juju (I believe), NewClient must use a default http client when validating hostnames, or many unit tests will fail.

The primary culprit is in the TestDataSuite: https://github.com/juju/juju/blob/04305f729f6a202e3b7c4cbdd5161eeb7e3fff82/environs/simplestreams/testing/testing.go#L604 and well as unit test calling SetRoundTripperFiles. This is where tests can replace the DefaultTransport RoundTripper and add schemes for test.

Unfortunately some of this is used in the JujuConnSuite as well as provisioner tests and difficult to replace.

Instead we should have more mocking of unit tests and use the httptest package more in environs/simplestreams.

Once the TestDataSuite can be removed, we should update http.NewClient accordingly and stop changing the DefaultTransport.

Revision history for this message
Heather Lanigan (hmlanigan) wrote :

Also investigate whether http.OutgoingAccessAllowed and installHTTPDialShim are really needed.

Revision history for this message
Canonical Juju QA Bot (juju-qa-bot) wrote :

This bug has not been updated in 2 years, so we're marking it Low importance. If you believe this is incorrect, please update the importance.

Changed in juju:
importance: Medium → Low
tags: added: expirebugs-bot
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.