Merge pull request juju#16596 from manadart/2.9-fix-bad-clouds-panic

juju#16596

This linked bug describes a situation where poorly formed YAML for local cloud definitions results in a panic.

The solution is simple - just use plain structs instead of pointers. This allows us to get serendipitous fall-back to zero values instead of panics for nil references.

## QA steps

- Test (for regression) that you can bootstrap to one of the clouds defined in _~/.local/share/juju/clouds.yaml_.
- Back-up your the _clouds.yaml_ file.
- Replace the contents of _clouds.yaml_ with:
```
clouds:
manual-cloud:
 type: manual
 endpoint: [email protected]
```
- `juju bootstrap manual-cloud manual`
- An error will result, but there will be no panic.

## Links

**Launchpad bug:** https://pad.lv/2039322

**Jira card:** JUJU-4975

cloud/clouds.go

@@ -5,7 +5,6 @@ package cloud
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"reflect"
 	"sort"
@@ -241,7 +240,7 @@ func (r Region) IsEmpty() bool {
 // unmarshalling.
 type cloudSet struct {
 	// Clouds is a map of cloud definitions, keyed on cloud name.
-	Clouds map[string]*cloud `yaml:"clouds"`
+	Clouds map[string]cloud `yaml:"clouds"`
 }
 
 // cloud is equivalent to Cloud, for marshalling and unmarshalling.
@@ -369,7 +368,7 @@ func JujuPublicCloudsPath() string {
 // are found, returns the fallback public cloud metadata.
 func PublicCloudMetadata(searchPath ...string) (result map[string]Cloud, fallbackUsed bool, err error) {
 	for _, file := range searchPath {
-		data, err := ioutil.ReadFile(file)
+		data, err := os.ReadFile(file)
 		if err != nil && os.IsNotExist(err) {
 			continue
 		}
@@ -388,7 +387,7 @@ func PublicCloudMetadata(searchPath ...string) (result map[string]Cloud, fallbac
 
 // ParseOneCloud parses the given yaml bytes into a single Cloud metadata.
 func ParseOneCloud(data []byte) (Cloud, error) {
-	c := &cloud{}
+	var c cloud
 	if err := yaml.Unmarshal(data, &c); err != nil {
 		return Cloud{}, errors.Annotate(err, "cannot unmarshal yaml cloud metadata")
 	}
@@ -444,7 +443,7 @@ func ParseCloudMetadata(data []byte) (map[string]Cloud, error) {
 		}
 	} else {
 		// Unable to coerce cloudSet, try to unmarshal into a map[string]*cloud
-		cloudMap := make(map[string]*cloud)
+		cloudMap := make(map[string]cloud)
 		if errCloudMap := yaml.Unmarshal(data, &cloudMap); errCloudMap != nil {
 			return nil, errors.Errorf("Invalid cloud metadata %s", yamlMap)
 		}
@@ -515,7 +514,7 @@ func IsSameCloudMetadata(meta1, meta2 map[string]Cloud) (bool, error) {
 
 // marshalCloudMetadata marshals the given clouds to YAML.
 func marshalCloudMetadata(cloudsMap map[string]Cloud) ([]byte, error) {
-	clouds := cloudSet{make(map[string]*cloud)}
+	clouds := cloudSet{make(map[string]cloud)}
 	for name, metadata := range cloudsMap {
 		clouds.Clouds[name] = cloudToInternal(metadata, false)
 	}
@@ -537,10 +536,10 @@ func UnmarshalCloud(in []byte) (Cloud, error) {
 	if err := yaml.Unmarshal(in, &internal); err != nil {
 		return Cloud{}, errors.Annotate(err, "cannot unmarshal yaml cloud metadata")
 	}
-	return cloudFromInternal(&internal), nil
+	return cloudFromInternal(internal), nil
 }
 
-func cloudToInternal(in Cloud, withName bool) *cloud {
+func cloudToInternal(in Cloud, withName bool) cloud {
 	var regions regions
 	for _, r := range in.Regions {
 		regions.Slice = append(regions.Slice, yaml.MapItem{
@@ -556,7 +555,7 @@ func cloudToInternal(in Cloud, withName bool) *cloud {
 	if !withName {
 		name = ""
 	}
-	return &cloud{
+	return cloud{
 		Name:              name,
 		Type:              in.Type,
 		HostCloudRegion:   in.HostCloudRegion,
@@ -573,7 +572,7 @@ func cloudToInternal(in Cloud, withName bool) *cloud {
 	}
 }
 
-func cloudFromInternal(in *cloud) Cloud {
+func cloudFromInternal(in cloud) Cloud {
 	var regions []Region
 	if len(in.Regions.Map) > 0 {
 		for _, item := range in.Regions.Slice {

cloud/clouds_test.go

@@ -276,6 +276,19 @@ clouds:
 	s.assertCompareClouds(c, metadata, false)
 }
 
+func (s *cloudSuite) TestMalformedYAMLNoPanic(_ *gc.C) {
+	// Note the bad indentation. This case was reported under LP:2039322.
+	metadata := `
+clouds:
+manual-cloud:
+    type: manual
+    endpoint: ubuntu@some-host-fqdn
+`[1:]
+
+	// We don't care about the result, just that there is no panic.
+	_, _ = cloud.ParseCloudMetadata([]byte(metadata))
+}
+
 func (s *cloudSuite) TestRegionNames(c *gc.C) {
 	regions := []cloud.Region{
 		{Name: "mars"},