Bug fixed

sdg0919 · Oct 3, 2013 · a50ff5e · a50ff5e
1 parent 5ca1511
commit a50ff5e
Show file tree

Hide file tree

Showing 8 changed files with 142 additions and 28 deletions.
diff --git a/downloader/downloader.go b/downloader/downloader.go
@@ -13,6 +13,7 @@ import (
 	"launchpad.net/tomb"
 
 	"launchpad.net/juju-core/log"
+	"launchpad.net/juju-core/utils"
 )
 
 // Status represents the status of a completed download.
@@ -25,15 +26,19 @@ type Status struct {
 
 // Download can download a file from the network.
 type Download struct {
-	tomb tomb.Tomb
-	done chan Status
+	tomb                           tomb.Tomb
+	done                           chan Status
+	disableSSLHostnameVerification bool
 }
 
-// New returns a new Download instance downloading from the given URL to
-// the given directory. If dir is empty, it defaults to os.TempDir().
-func New(url, dir string) *Download {
+// New returns a new Download instance downloading from the given URL
+// to the given directory. If dir is empty, it defaults to
+// os.TempDir(). If disableSSLHostnameVerification is true then a non-
+// validating http client will be used.
+func New(url, dir string, disableSSLHostnameVerification bool) *Download {
 	d := &Download{
 		done: make(chan Status),
+		disableSSLHostnameVerification: disableSSLHostnameVerification,
 	}
 	go d.run(url, dir)
 	return d
@@ -54,7 +59,7 @@ func (d *Download) Done() <-chan Status {
 
 func (d *Download) run(url, dir string) {
 	defer d.tomb.Done()
-	file, err := download(url, dir)
+	file, err := download(url, dir, d.disableSSLHostnameVerification)
 	if err != nil {
 		err = fmt.Errorf("cannot download %q: %v", url, err)
 	}
@@ -69,7 +74,7 @@ func (d *Download) run(url, dir string) {
 	}
 }
 
-func download(url, dir string) (file *os.File, err error) {
+func download(url, dir string, disableSSLHostnameVerification bool) (file *os.File, err error) {
 	if dir == "" {
 		dir = os.TempDir()
 	}
@@ -83,7 +88,11 @@ func download(url, dir string) (file *os.File, err error) {
 		}
 	}()
 	// TODO(rog) make the download operation interruptible.
-	resp, err := http.Get(url)
+	client := http.DefaultClient
+	if disableSSLHostnameVerification {
+		client = utils.GetNonValidatingHTTPClient()
+	}
+	resp, err := client.Get(url)
 	if err != nil {
 		return nil, err
 	}

diff --git a/downloader/downloader_test.go b/downloader/downloader_test.go
@@ -48,10 +48,10 @@ func Test(t *stdtesting.T) {
 	gc.TestingT(t)
 }
 
-func (s *suite) TestDownload(c *gc.C) {
+func (s *suite) testDownload(c *gc.C, disableSSLHostnameVerification bool) {
 	tmp := c.MkDir()
 	testing.Server.Response(200, nil, []byte("archive"))
-	d := downloader.New(s.URL("/archive.tgz"), tmp)
+	d := downloader.New(s.URL("/archive.tgz"), tmp, disableSSLHostnameVerification)
 	status := <-d.Done()
 	c.Assert(status.Err, gc.IsNil)
 	c.Assert(status.File, gc.NotNil)
@@ -63,17 +63,25 @@ func (s *suite) TestDownload(c *gc.C) {
 	assertFileContents(c, status.File, "archive")
 }
 
+func (s *suite) TestDownloadWithoutDisablingSSLHostnameVerification(c *gc.C) {
+	s.testDownload(c, false)
+}
+
+func (s *suite) TestDownloadWithDisablingSSLHostnameVerification(c *gc.C) {
+	s.testDownload(c, true)
+}
+
 func (s *suite) TestDownloadError(c *gc.C) {
 	testing.Server.Response(404, nil, nil)
-	d := downloader.New(s.URL("/archive.tgz"), c.MkDir())
+	d := downloader.New(s.URL("/archive.tgz"), c.MkDir(), false)
 	status := <-d.Done()
 	c.Assert(status.File, gc.IsNil)
 	c.Assert(status.Err, gc.ErrorMatches, `cannot download ".*": bad http response: 404 Not Found`)
 }
 
 func (s *suite) TestStopDownload(c *gc.C) {
 	tmp := c.MkDir()
-	d := downloader.New(s.URL("/x.tgz"), tmp)
+	d := downloader.New(s.URL("/x.tgz"), tmp, false)
 	d.Stop()
 	select {
 	case status := <-d.Done():

diff --git a/state/api/params/internal.go b/state/api/params/internal.go
@@ -67,6 +67,21 @@ type StringResults struct {
 	Results []StringResult
 }
 
+// CharmArchiveURLResult holds a charm archive (bunle) URL, a
+// DisableSSLHostnameVerification flag or an error.
+type CharmArchiveURLResult struct {
+	Error                          *Error
+	Result                         string
+	DisableSSLHostnameVerification bool
+}
+
+// CharmArchiveURLResults holds the bulk operation result of an API
+// call that returns a charm archive (bundle) URL, a
+// DisableSSLHostnameVerification flag or an error.
+type CharmArchiveURLResults struct {
+	Results []CharmArchiveURLResult
+}
+
 // ResolvedModeResult holds a resolved mode or an error.
 type ResolvedModeResult struct {
 	Error *Error

diff --git a/state/api/uniter/charm.go b/state/api/uniter/charm.go
@@ -50,7 +50,7 @@ func (c *Charm) getArchiveInfo(apiCall string) (string, error) {
 }
 
 // ArchiveURL returns the url to the charm archive (bundle) in the
-// provider storage.
+// provider storage, and DisableSSLHostnameVerification flag.
 //
 // NOTE: This differs from state.Charm.BundleURL() by returning an
 // error as well, because it needs to make an API call. It's also
@@ -59,12 +59,27 @@ func (c *Charm) getArchiveInfo(apiCall string) (string, error) {
 // TODO(dimitern): 2013-09-06 bug 1221834
 // Cache the result after getting it once for the same charm URL,
 // because it's immutable.
-func (c *Charm) ArchiveURL() (*url.URL, error) {
-	charmURL, err := c.getArchiveInfo("CharmArchiveURL")
+func (c *Charm) ArchiveURL() (*url.URL, bool, error) {
+	var results params.CharmArchiveURLResults
+	args := params.CharmURLs{
+		URLs: []params.CharmURL{{URL: c.url}},
+	}
+	err := c.st.caller.Call("Uniter", "", "CharmArchiveURL", args, &results)
+	if err != nil {
+		return nil, false, err
+	}
+	if len(results.Results) != 1 {
+		return nil, false, fmt.Errorf("expected one result, got %d", len(results.Results))
+	}
+	result := results.Results[0]
+	if result.Error != nil {
+		return nil, false, result.Error
+	}
+	archiveURL, err := url.Parse(result.Result)
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
-	return url.Parse(charmURL)
+	return archiveURL, result.DisableSSLHostnameVerification, nil
 }
 
 // ArchiveSha256 returns the SHA256 digest of the charm archive

diff --git a/state/api/uniter/charm_test.go b/state/api/uniter/charm_test.go
@@ -6,7 +6,9 @@ package uniter_test
 import (
 	gc "launchpad.net/gocheck"
 
+	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/state/api/uniter"
+	jc "launchpad.net/juju-core/testing/checkers"
 )
 
 type charmSuite struct {
@@ -44,9 +46,26 @@ func (s *charmSuite) TestURL(c *gc.C) {
 }
 
 func (s *charmSuite) TestArchiveURL(c *gc.C) {
-	archiveURL, err := s.apiCharm.ArchiveURL()
+	archiveURL, disableSSLHostnameVerification, err := s.apiCharm.ArchiveURL()
 	c.Assert(err, gc.IsNil)
 	c.Assert(archiveURL, gc.DeepEquals, s.wordpressCharm.BundleURL())
+	c.Assert(disableSSLHostnameVerification, jc.IsFalse)
+
+	// Change the environment config to have
+	// "ssl-hostname-verification" false.
+	envConfig, err := s.State.EnvironConfig()
+	c.Assert(err, gc.IsNil)
+	attrs := envConfig.AllAttrs()
+	attrs["ssl-hostname-verification"] = false
+	newConfig, err := config.New(config.NoDefaults, attrs)
+	c.Assert(err, gc.IsNil)
+	err = s.State.SetEnvironConfig(newConfig)
+	c.Assert(err, gc.IsNil)
+
+	archiveURL, disableSSLHostnameVerification, err = s.apiCharm.ArchiveURL()
+	c.Assert(err, gc.IsNil)
+	c.Assert(archiveURL, gc.DeepEquals, s.wordpressCharm.BundleURL())
+	c.Assert(disableSSLHostnameVerification, jc.IsTrue)
 }
 
 func (s *charmSuite) TestArchiveSha256(c *gc.C) {

diff --git a/state/apiserver/uniter/uniter.go b/state/apiserver/uniter/uniter.go
@@ -556,11 +556,21 @@ func (u *UniterAPI) WatchServiceRelations(args params.Entities) (params.StringsW
 }
 
 // CharmArchiveURL returns the URL, corresponding to the charm archive
-// (bundle) in the provider storage for each given charm URL.
-func (u *UniterAPI) CharmArchiveURL(args params.CharmURLs) (params.StringResults, error) {
-	result := params.StringResults{
-		Results: make([]params.StringResult, len(args.URLs)),
+// (bundle) in the provider storage for each given charm URL, along
+// with the DisableSSLHostnameVerification flag.
+func (u *UniterAPI) CharmArchiveURL(args params.CharmURLs) (params.CharmArchiveURLResults, error) {
+	result := params.CharmArchiveURLResults{
+		Results: make([]params.CharmArchiveURLResult, len(args.URLs)),
+	}
+	// Get the SSL hostname verification environment setting.
+	envConfig, err := u.st.EnvironConfig()
+	if err != nil {
+		return result, err
 	}
+	// SSLHostnameVerification defaults to true, so we need to
+	// invert that, for backwards-compatibility (older versions
+	// will have DisableSSLHostnameVerification: false by default).
+	disableSSLHostnameVerification := !envConfig.SSLHostnameVerification()
 	for i, arg := range args.URLs {
 		curl, err := charm.ParseURL(arg.URL)
 		if err != nil {
@@ -573,6 +583,7 @@ func (u *UniterAPI) CharmArchiveURL(args params.CharmURLs) (params.StringResults
 			}
 			if err == nil {
 				result.Results[i].Result = sch.BundleURL().String()
+				result.Results[i].DisableSSLHostnameVerification = disableSSLHostnameVerification
 			}
 		}
 		result.Results[i].Error = common.ServerError(err)

diff --git a/state/apiserver/uniter/uniter_test.go b/state/apiserver/uniter/uniter_test.go
@@ -9,6 +9,7 @@ import (
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/charm"
+	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/errors"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju/testing"
@@ -839,11 +840,44 @@ func (s *uniterSuite) TestCharmArchiveURL(c *gc.C) {
 	}}
 	result, err := s.uniter.CharmArchiveURL(args)
 	c.Assert(err, gc.IsNil)
-	c.Assert(result, gc.DeepEquals, params.StringResults{
-		Results: []params.StringResult{
+	c.Assert(result, gc.DeepEquals, params.CharmArchiveURLResults{
+		Results: []params.CharmArchiveURLResult{
 			{Error: apiservertesting.ErrUnauthorized},
-			{Result: s.wpCharm.BundleURL().String()},
-			{Result: dummyCharm.BundleURL().String()},
+			{
+				Result: s.wpCharm.BundleURL().String(),
+				DisableSSLHostnameVerification: false,
+			},
+			{
+				Result: dummyCharm.BundleURL().String(),
+				DisableSSLHostnameVerification: false,
+			},
+		},
+	})
+
+	// Change the environment config to have
+	// "ssl-hostname-verification" false.
+	envConfig, err := s.State.EnvironConfig()
+	c.Assert(err, gc.IsNil)
+	attrs := envConfig.AllAttrs()
+	attrs["ssl-hostname-verification"] = false
+	newConfig, err := config.New(config.NoDefaults, attrs)
+	c.Assert(err, gc.IsNil)
+	err = s.State.SetEnvironConfig(newConfig)
+	c.Assert(err, gc.IsNil)
+
+	result, err = s.uniter.CharmArchiveURL(args)
+	c.Assert(err, gc.IsNil)
+	c.Assert(result, gc.DeepEquals, params.CharmArchiveURLResults{
+		Results: []params.CharmArchiveURLResult{
+			{Error: apiservertesting.ErrUnauthorized},
+			{
+				Result: s.wpCharm.BundleURL().String(),
+				DisableSSLHostnameVerification: true,
+			},
+			{
+				Result: dummyCharm.BundleURL().String(),
+				DisableSSLHostnameVerification: true,
+			},
 		},
 	})
 }

diff --git a/worker/uniter/charm/charm.go b/worker/uniter/charm/charm.go
@@ -48,7 +48,7 @@ func (d *BundlesDir) Read(sch *uniter.Charm, abort <-chan struct{}) (*charm.Bund
 // hash, then copies it into the directory. If a value is received on abort, the
 // download will be stopped.
 func (d *BundlesDir) download(sch *uniter.Charm, abort <-chan struct{}) (err error) {
-	archiveURL, err := sch.ArchiveURL()
+	archiveURL, disableSSLHostnameVerification, err := sch.ArchiveURL()
 	if err != nil {
 		return err
 	}
@@ -59,7 +59,10 @@ func (d *BundlesDir) download(sch *uniter.Charm, abort <-chan struct{}) (err err
 	}
 	aurl := archiveURL.String()
 	log.Infof("worker/uniter/charm: downloading %s from %s", sch.URL(), aurl)
-	dl := downloader.New(aurl, dir)
+	if disableSSLHostnameVerification {
+		log.Infof("worker/uniter/charm: SSL hostname verification disabled")
+	}
+	dl := downloader.New(aurl, dir, disableSSLHostnameVerification)
 	defer dl.Stop()
 	for {
 		select {