Merge pull request juju#18294 from nvinuesa/juju-6344

juju#18294

Before this patch we weren't able to bootstrap on MAAS when configured for TLS with a self-signed certificate.
We now add the posibility to take the `skip-tls-verify` cloud spec field into account and create an insecure TLS http client for the maas controller connection in that case.

<!-- 
The PR title should match: <type>(optional <scope>): <description>.

Please also ensure all commits in this PR comply with our conventional commits specification:
https://docs.google.com/document/d/1SYUo9G7qZ_jdoVXpUVamS5VCgHmtZ0QA-wZxKoMS-C0 
-->

<!-- Why this change is needed and what it does. -->

## Checklist

<!-- If an item is not applicable, use `~strikethrough~`. -->

- [X] Code style: imports ordered, good names, simple structure, etc
- [X] Comments saying why design decisions were made
- [X] Go unit tests, with comments saying what you're testing
- [ ] ~[Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~
- [ ] ~[doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

_Note: you should wait until maas downloads the ubuntu image before bootstrapping, otherwise it won't be able to create a machine._
The steps will include running a local MAAS inside multipass (follow https://maas.io/docs/maas-in-thirty-minutes), and enable TLS with a self signed certificate. 
Then creating a maas juju cloud with `skip-tls-verify: true`:

```
wget -qO- https://raw.githubusercontent.com/canonical/maas-multipass/main/maas.yml \n | multipass launch --name maas -c4 -m8GB -d32GB --cloud-init -

# once the VM is up:

multipass shell maas
```
the following steps are run inside the VM:
```
sudo bash -c 'cat <<EOF > ~/san.conf
[req]
default_bits = 2048
distinguished_name = req_distinguished_name
req_extensions = req_ext
x509_extensions = v3_req
prompt = no

[req_distinguished_name]
countryName = XX
stateOrProvinceName = N/A
localityName = N/A
organizationName = Self-signed certificate
commonName = 120.0.0.1: Self-signed certificate

[req_ext]
subjectAltName = @alt_names

[v3_req]
subjectAltName = @alt_names

[alt_names]
IP.1 = 10.254.213.97
```
_Note: make sure to replace the last IP with the IP of your maas VM (the first of the two IPs you see when `multipass ls`!_

Then create the cert and move it to `/var/snap/maas/common/cert` (it needs to be accessible to the maas snap):
```
openssl req -x509 -nodes -days 730 -newkey rsa:2048 -keyout key.pem -out cert.pem -config san.cnf
sudo mkdir /var/snap/maas/common/cert
sudo mv key.pem /var/snap/maas/common/cert 
sudo mv cert.pem /var/snap/maas/common/cert 
sudo chown root:root /var/snap/maas/common/cert/*
```
and now you can enable TLS with that cert:
```
sudo maas config-tls enable /var/snap/maas/common/cert/key.pem /var/snap/maas/common/cert/cert.pem
```
also retrieve the apikey needed for the juju credential:
```
sudo maas apikey --username admin
euGNK3UyF7H247LdkY:p9nQaDe4xgxZRzXsRZ:rEqtjsjGHCSUWSXERm4qCXfXGrWuXEPx
```
Now you can quit the multipass shell and create the maas cloud for juju with this yaml:
```
 maas-multipass:
 type: maas
 auth-types: [oauth1]
 endpoint: https://10.254.213.97:5443/MAAS
 skip-tls-verify: true
```
and the credential:
```
 maas-multipass:
 foo:
 auth-type: oauth1
 maas-oauth: euGNK3UyF7H247LdkY:p9nQaDe4xgxZRzXsRZ:rEqtjsjGHCSUWSXERm4qCXfXGrWuXEPx
```

Now you can bootstrap and no errors seen:
```
juju bootstrap maas-multipass --debug
```

If you remove the `skip-tls-verify: true` field from the cloud spec, then you should see the error:
```
... creating MAAS environ: Get "https://10.254.213.97:5443/MAAS/api/2.0/version/": tls: failed to verify certificate: x509: certificate signed by unknown authority
```
## Links

**Launchpad bug:** https://bugs.launchpad.net/juju/+bug/2072653

**Jira card:** JUJU-6344

provider/maas/config_test.go

@@ -38,7 +38,7 @@ func newConfig(values map[string]interface{}) (*maasModelConfig, error) {
 
 func (s *configSuite) SetUpTest(c *gc.C) {
 	s.BaseSuite.SetUpTest(c)
-	mockGetController := func(string, string) (gomaasapi.Controller, error) {
+	mockGetController := func(gomaasapi.ControllerArgs) (gomaasapi.Controller, error) {
 		return nil, gomaasapi.NewUnsupportedVersionError("oops")
 	}
 	s.PatchValue(&GetMAASController, mockGetController)

provider/maas/environ.go

@@ -5,6 +5,7 @@ package maas
 
 import (
 	stdcontext "context"
+	"crypto/tls"
 	"fmt"
 	"net/http"
 	"regexp"
@@ -51,6 +52,7 @@ var defaultShortRetryStrategy = retry.CallArgs{
 	Delay:       200 * time.Millisecond,
 	MaxDuration: 5 * time.Second,
 }
+
 var defaultLongRetryStrategy = retry.CallArgs{
 	Clock:       clock.WallClock,
 	Delay:       10 * time.Second,
@@ -62,11 +64,8 @@ var (
 	GetMAASController    = getMAASController
 )
 
-func getMAASController(maasServer, apiKey string) (gomaasapi.Controller, error) {
-	return gomaasapi.NewController(gomaasapi.ControllerArgs{
-		BaseURL: maasServer,
-		APIKey:  apiKey,
-	})
+func getMAASController(args gomaasapi.ControllerArgs) (gomaasapi.Controller, error) {
+	return gomaasapi.NewController(args)
 }
 
 type maasEnviron struct {
@@ -104,8 +103,10 @@ type maasEnviron struct {
 	longRetryStrategy  retry.CallArgs
 }
 
-var _ environs.Environ = (*maasEnviron)(nil)
-var _ environs.Networking = (*maasEnviron)(nil)
+var (
+	_ environs.Environ    = (*maasEnviron)(nil)
+	_ environs.Networking = (*maasEnviron)(nil)
+)
 
 // Capabilities is an alias for a function that gets
 // the capabilities of a MAAS installation.
@@ -261,7 +262,22 @@ func (env *maasEnviron) SetCloudSpec(_ stdcontext.Context, spec environscloudspe
 	}
 
 	apiVersion := apiVersion2
-	controller, err := GetMAASController(maasServer, maasOAuth)
+	args := gomaasapi.ControllerArgs{
+		BaseURL: maasServer,
+		APIKey:  maasOAuth,
+	}
+	// If the user has specified to skip TLS verification, we need to
+	// add a new http client with insecure TLS (skip verify).
+	if spec.SkipTLSVerify {
+		args.HTTPClient = &http.Client{
+			Transport: &http.Transport{
+				TLSClientConfig: &tls.Config{
+					InsecureSkipVerify: true,
+				},
+			},
+		}
+	}
+	controller, err := GetMAASController(args)
 	if err != nil {
 		return errors.Trace(err)
 	}
@@ -630,7 +646,6 @@ func (env *maasEnviron) acquireNode(
 		acquireParams.SystemId = systemId
 	}
 	machine, constraintMatches, err := env.maasController.AllocateMachine(acquireParams)
-
 	if err != nil {
 		common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 		return nil, errors.Trace(err)
@@ -654,7 +669,6 @@ func (env *maasEnviron) StartInstance(
 	ctx context.ProviderCallContext,
 	args environs.StartInstanceParams,
 ) (_ *environs.StartInstanceResult, err error) {
-
 	availabilityZone := args.AvailabilityZone
 	var nodeName, systemId string
 	if args.Placement != "" {
@@ -1019,7 +1033,6 @@ func (env *maasEnviron) StopInstances(ctx context.ProviderCallContext, ids ...in
 		return errors.Trace(err)
 	}
 	return common.RemoveStateInstances(env.Storage(), ids...)
-
 }
 
 // Instances returns the instances.Instance objects corresponding to the given

provider/maas/environprovider_test.go

@@ -59,6 +59,19 @@ func (s *EnvironProviderSuite) TestPrepareConfig(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 }
 
+func (s *EnvironProviderSuite) TestPrepareConfigSkipTLSVerify(c *gc.C) {
+	attrs := testing.FakeConfig().Merge(testing.Attrs{"type": "maas"})
+	config, err := config.New(config.NoDefaults, attrs)
+	c.Assert(err, jc.ErrorIsNil)
+	cloud := s.cloudSpec()
+	cloud.SkipTLSVerify = true
+	_, err = providerInstance.PrepareConfig(environs.PrepareConfigParams{
+		Config: config,
+		Cloud:  cloud,
+	})
+	c.Assert(err, jc.ErrorIsNil)
+}
+
 func (s *EnvironProviderSuite) TestPrepareConfigInvalidOAuth(c *gc.C) {
 	attrs := testing.FakeConfig().Merge(testing.Attrs{"type": "maas"})
 	config, err := config.New(config.NoDefaults, attrs)
@@ -108,7 +121,7 @@ func createTempFile(c *gc.C, content []byte) string {
 	defer file.Close()
 	c.Assert(err, jc.ErrorIsNil)
 	filename := file.Name()
-	err = os.WriteFile(filename, content, 0644)
+	err = os.WriteFile(filename, content, 0o644)
 	c.Assert(err, jc.ErrorIsNil)
 	return filename
 }

provider/maas/maas_environ_whitebox_test.go

@@ -91,6 +91,12 @@ func (suite *maasEnvironSuite) TestNewEnvironWithController(c *gc.C) {
 	c.Assert(env, gc.NotNil)
 }
 
+func (suite *maasEnvironSuite) TestNewEnvironWithControllerSkipTLSVerify(c *gc.C) {
+	env, err := suite.getEnvWithServer(c)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(env, gc.NotNil)
+}
+
 func (suite *maasEnvironSuite) injectControllerWithSpacesAndCheck(c *gc.C, spaces []gomaasapi.Space, expected gomaasapi.AllocateMachineArgs) (*maasEnviron, *fakeController) {
 	machine := newFakeMachine("Bruce Sterling", arch.HostArch(), "")
 	return suite.injectControllerWithMachine(c, machine, spaces, expected)
@@ -455,7 +461,8 @@ func (suite *maasEnvironSuite) TestAcquireNodePassedAgentName(c *gc.C) {
 	suite.injectController(&fakeController{
 		allocateMachineArgsCheck: func(args gomaasapi.AllocateMachineArgs) {
 			c.Assert(args, gc.DeepEquals, gomaasapi.AllocateMachineArgs{
-				AgentName: env.Config().UUID()})
+				AgentName: env.Config().UUID(),
+			})
 		},
 		allocateMachine: &fakeMachine{
 			systemID:     "Bruce Sterling",
@@ -1742,6 +1749,7 @@ func makeFakeSubnet(id int) fakeSubnet {
 		cidr:    fmt.Sprintf("10.20.%d.0/24", 16+id),
 	}
 }
+
 func (suite *maasEnvironSuite) TestAllocateContainerAddressesMachinesError(c *gc.C) {
 	var env *maasEnviron
 	subnet := makeFakeSubnet(3)

provider/maas/maas_test.go

@@ -31,7 +31,7 @@ type maasSuite struct {
 }
 
 func (suite *maasSuite) injectController(controller gomaasapi.Controller) {
-	mockGetController := func(maasServer, apiKey string) (gomaasapi.Controller, error) {
+	mockGetController := func(args gomaasapi.ControllerArgs) (gomaasapi.Controller, error) {
 		return controller, nil
 	}
 	suite.PatchValue(&GetMAASController, mockGetController)
@@ -186,6 +186,7 @@ func (c *fakeController) StaticRoutes() ([]gomaasapi.StaticRoute, error) {
 	}
 	return c.staticRoutes, nil
 }
+
 func (c *fakeController) Files(prefix string) ([]gomaasapi.File, error) {
 	c.MethodCall(c, "Files", prefix)
 	return c.files, c.NextErr()