Merge pull request #12868 from ycliuhw/merge-2.8

#12868 Merge 2.8 to 2.9: - Fix for LP1921557 sni in Juju login. #12854 - Ensure assess-upgrade-series does not report started prematuremly #12858 - Fix/lp 1923051 #12862 - Fix/lp 1923561 #12867 no conflicts
juju · Apr 14, 2021 · 9f34465 · 9f34465
2 parents 1ede980 + 805d593
commit 9f34465
Show file tree

Hide file tree

Showing 14 changed files with 216 additions and 38 deletions.
diff --git a/acceptancetests/assess_upgrade_series.py b/acceptancetests/assess_upgrade_series.py
@@ -20,6 +20,7 @@
     add_basic_testing_arguments,
     configure_logging,
     JujuAssertionError,
+    wait_for_port,
 )
 
 __metaclass__ = type
@@ -81,8 +82,9 @@ def reboot_machine(client, machine):
             raise e
         log.info("Ignoring `juju ssh` exit status after triggering reboot")
 
-    log.info("wait_for_started()")
-    client.wait_for_started()
+    log.info("waiting for reboot")
+    hostname = client.get_status().get_machine_dns_name(machine)
+    wait_for_port(hostname, 22, timeout=600)
 
 
 def assert_correct_series(client, machine, expected):

diff --git a/acceptancetests/jujupy/client.py b/acceptancetests/jujupy/client.py
@@ -1743,7 +1743,7 @@ def get_controller_member_status(info_dict):
         return info_dict.get('controller-member-status')
 
     def wait_for_ha(self, timeout=1200, start=None, quorum=3):
-        """Wait for voiting to be enabled.
+        """Wait for voting to be enabled.
 
         May only be called on a controller client."""
         if self.env.environment != self.get_controller_model_name():
@@ -1767,7 +1767,7 @@ def status_to_ha(status):
         self._wait_for_status(reporter, status_to_ha, VotingNotEnabled,
                               timeout=timeout, start=start)
         # XXX sinzui 2014-12-04: bug 1399277 happens because
-        # juju claims HA is ready when the monogo replica sets
+        # juju claims HA is ready when the mongo replica sets
         # are not. Juju is not fully usable. The replica set
         # lag might be 5 minutes.
         self._backend.pause(300)

diff --git a/apiserver/facades/controller/caasoperatorprovisioner/provisioner_test.go b/apiserver/facades/controller/caasoperatorprovisioner/provisioner_test.go
@@ -246,7 +246,7 @@ func (s *CAASProvisionerSuite) TestIssueOperatorCertificate(c *gc.C) {
 		[]byte(certInfo.PrivateKey)...))
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(len(signers), gc.Equals, 1)
-	c.Assert(len(certs), gc.Equals, 1)
+	c.Assert(len(certs), gc.Equals, 2)
 
 	roots := x509.NewCertPool()
 	ok := roots.AppendCertsFromPEM([]byte(certInfo.CACert))

diff --git a/caas/kubernetes/provider/k8s.go b/caas/kubernetes/provider/k8s.go
@@ -1064,13 +1064,18 @@ func (k *kubernetesClient) EnsureService(
 	if numUnits == 0 {
 		return k.deleteAllPods(appName, deploymentName)
 	}
+	if params.PodSpec != nil && len(params.RawK8sSpec) > 0 {
+		// This should never happen.
+		return errors.NotValidf("both pod spec and raw k8s spec provided")
+	}
 
 	if params.PodSpec != nil {
 		return k.ensureService(appName, deploymentName, statusCallback, params, numUnits, config)
-	} else if len(params.RawK8sSpec) > 0 {
+	}
+	if len(params.RawK8sSpec) > 0 {
 		return k.applyRawK8sSpec(appName, deploymentName, statusCallback, params, numUnits, config)
 	}
-	return errors.NewNotSupported(nil, "currently only k8s-raw-set and k8s-spec-set are supported")
+	return nil
 }
 
 func (k *kubernetesClient) ensureService(

diff --git a/caas/kubernetes/provider/k8s_test.go b/caas/kubernetes/provider/k8s_test.go
@@ -2055,6 +2055,37 @@ func (s *K8sBrokerSuite) TestEnsureServiceNoUnits(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 }
 
+func (s *K8sBrokerSuite) TestEnsureServiceNoSpecProvided(c *gc.C) {
+	ctrl := s.setupController(c)
+	defer ctrl.Finish()
+
+	gomock.InOrder(
+		s.mockStatefulSets.EXPECT().Get(gomock.Any(), "juju-operator-app-name", v1.GetOptions{}).
+			Return(nil, s.k8sNotFoundError()),
+	)
+
+	params := &caas.ServiceParams{}
+	err := s.broker.EnsureService("app-name", func(_ string, _ status.Status, _ string, _ map[string]interface{}) error { return nil }, params, 1, nil)
+	c.Assert(err, jc.ErrorIsNil)
+}
+
+func (s *K8sBrokerSuite) TestEnsureServiceBothPodSpecAndRawK8sSpecProvided(c *gc.C) {
+	ctrl := s.setupController(c)
+	defer ctrl.Finish()
+
+	gomock.InOrder(
+		s.mockStatefulSets.EXPECT().Get(gomock.Any(), "juju-operator-app-name", v1.GetOptions{}).
+			Return(nil, s.k8sNotFoundError()),
+	)
+
+	params := &caas.ServiceParams{
+		PodSpec:    getBasicPodspec(),
+		RawK8sSpec: `fake raw spec`,
+	}
+	err := s.broker.EnsureService("app-name", func(_ string, _ status.Status, _ string, _ map[string]interface{}) error { return nil }, params, 1, nil)
+	c.Assert(err, gc.ErrorMatches, `both pod spec and raw k8s spec provided not valid`)
+}
+
 func (s *K8sBrokerSuite) TestEnsureServiceNoStorage(c *gc.C) {
 	ctrl := s.setupController(c)
 	defer ctrl.Finish()

diff --git a/cmd/juju/caas/eks.go b/cmd/juju/caas/eks.go
@@ -105,11 +105,45 @@ func (e *eks) interactiveParams(ctx *cmd.Context, p *clusterParams) (*clusterPar
 	return p, nil
 }
 
-type eksDetail struct {
+type eksDetailLegacy struct {
 	Name   string `json:"name"`
 	Region string `json:"region"`
 }
 
+type eksStatus struct {
+	EKSCTLCreated string `json:"eksctlCreated"`
+}
+
+type eksDetail struct {
+	MetaData eksDetailLegacy `json:"metadata"`
+	Status   eksStatus       `json:"status"`
+}
+
+func getClusterNames(data []byte) (out []string, err error) {
+	var clusterDetails []eksDetail
+	if err := json.Unmarshal(data, &clusterDetails); err == nil {
+		for _, detail := range clusterDetails {
+			if len(detail.MetaData.Name) > 0 {
+				out = append(out, detail.MetaData.Name)
+			}
+		}
+	}
+	if len(out) > 0 {
+		return out, nil
+	}
+	var clusterDetailsLegacy []eksDetailLegacy
+	if err := json.Unmarshal(data, &clusterDetailsLegacy); err != nil {
+		return nil, errors.Trace(err)
+	}
+	for _, detail := range clusterDetailsLegacy {
+		if len(detail.Name) > 0 {
+			out = append(out, detail.Name)
+		}
+	}
+
+	return out, nil
+}
+
 func (e *eks) listClusters(region string) (clusters []string, err error) {
 	cmd := []string{
 		e.tool, "get", "cluster", "--region", region, "-o", "json",
@@ -121,14 +155,5 @@ func (e *eks) listClusters(region string) (clusters []string, err error) {
 	if result.Code != 0 {
 		return nil, errors.New(string(result.Stderr))
 	}
-
-	var eksClusters []eksDetail
-	if err := json.Unmarshal(result.Stdout, &eksClusters); err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	for _, item := range eksClusters {
-		clusters = append(clusters, item.Name)
-	}
-	return clusters, nil
+	return getClusterNames(result.Stdout)
 }
diff --git a/cmd/juju/caas/eks_test.go b/cmd/juju/caas/eks_test.go
@@ -159,7 +159,7 @@ Enter region:
 	c.Check(cmdtesting.Stdout(ctx), gc.Equals, expected)
 }
 
-func (s *eksSuite) TestInteractiveParamMultiClusters(c *gc.C) {
+func (s *eksSuite) TestInteractiveParamMultiClustersLegacyCLI(c *gc.C) {
 	ctrl := gomock.NewController(c)
 	defer ctrl.Finish()
 
@@ -216,6 +216,73 @@ Select cluster [mycluster]:
 	})
 }
 
+func (s *eksSuite) TestInteractiveParamMultiClusters(c *gc.C) {
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+
+	mockRunner := mocks.NewMockCommandRunner(ctrl)
+	eksCMD := &eks{
+		tool:          "eksctl",
+		CommandRunner: mockRunner,
+	}
+	clusterJSONResp := `
+[
+    {
+        "metadata": {
+            "name": "nw-deploy-kubeflow-1272",
+            "region": "ap-southeast-2"
+        },
+        "status": {
+            "eksctlCreated": "True"
+        }
+    },
+    {
+        "metadata": {
+            "name": "k1",
+            "region": "ap-southeast-2"
+        },
+        "status": {
+            "eksctlCreated": "True"
+        }
+    }
+]`
+
+	gomock.InOrder(
+		mockRunner.EXPECT().RunCommands(exec.RunParams{
+			Commands:    "eksctl get cluster --region ap-southeast-2 -o json",
+			Environment: os.Environ(),
+		}).
+			Return(&exec.ExecResponse{
+				Code:   0,
+				Stdout: []byte(clusterJSONResp),
+			}, nil),
+	)
+	stdin := strings.NewReader("ap-southeast-2\nk1\n")
+	out := &bytes.Buffer{}
+	ctx := &cmd.Context{
+		Dir:    c.MkDir(),
+		Stdout: out,
+		Stderr: ioutil.Discard,
+		Stdin:  stdin,
+	}
+	expected := `
+Enter region: 
+Available Clusters In Ap-Southeast-2
+  nw-deploy-kubeflow-1272
+  k1
+
+Select cluster [nw-deploy-kubeflow-1272]: 
+`[1:]
+
+	outParams, err := eksCMD.interactiveParams(ctx, &clusterParams{})
+	c.Check(err, jc.ErrorIsNil)
+	c.Check(cmdtesting.Stdout(ctx), gc.Equals, expected)
+	c.Assert(outParams, jc.DeepEquals, &clusterParams{
+		name:   "k1",
+		region: "ap-southeast-2",
+	})
+}
+
 func (s *eksSuite) TestEnsureExecutable(c *gc.C) {
 	ctrl := gomock.NewController(c)
 	defer ctrl.Finish()

diff --git a/cmd/juju/user/login.go b/cmd/juju/user/login.go
@@ -8,6 +8,7 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
+	"net"
 	"net/http"
 	"os"
 	"strings"
@@ -362,10 +363,16 @@ func (c *loginCommand) publicControllerLogin(
 			dialOpts.BakeryClient.AddInteractor(i)
 		}
 
+		sniHost, _, err := net.SplitHostPort(host)
+		if err != nil {
+			return nil, errors.Annotatef(err, "getting sni host from host %q", host)
+		}
+
 		return apiOpen(&c.CommandBase, &api.Info{
-			Tag:      tag,
-			Password: d.Password,
-			Addrs:    []string{host},
+			Tag:         tag,
+			Password:    d.Password,
+			Addrs:       []string{host},
+			SNIHostName: sniHost,
 		}, dialOpts)
 	}
 	conn, accountDetails, err := c.login(ctx, currentAccountDetails, dial)

diff --git a/pki/authority.go b/pki/authority.go
@@ -18,7 +18,8 @@ import (
 )
 
 const (
-	DefaultLeafGroup = "controller"
+	DefaultLeafGroup      = "controller"
+	ControllerIPLeafGroup = "controllerip"
 )
 
 // Authority represents a secure means of issuing groups of common interest
@@ -80,6 +81,14 @@ func (a *DefaultAuthority) Chain() []*x509.Certificate {
 	return a.authority.Chain()
 }
 
+func (a *DefaultAuthority) ChainWithAuthority() []*x509.Certificate {
+	chain := a.authority.Chain()
+	if chain == nil {
+		chain = []*x509.Certificate{}
+	}
+	return append(chain, a.authority.Certificate())
+}
+
 // leafMaker is responsible for providing a method to make new leafs after
 // request signing.
 func (a *DefaultAuthority) leafMaker(groupKey string) LeafMaker {
@@ -104,11 +113,11 @@ func (a *DefaultAuthority) LeafRequestForGroup(group string) LeafRequest {
 	defer a.leafSignerMutex.Unlock()
 	if a.leafSigner != nil {
 		return NewDefaultLeafRequestWithSigner(subject, a.leafSigner,
-			NewDefaultRequestSigner(a.Certificate(), a.Chain(), a.Signer()),
+			NewDefaultRequestSigner(a.Certificate(), a.ChainWithAuthority(), a.Signer()),
 			a.leafMaker(groupKey))
 	}
 	return NewDefaultLeafRequest(subject,
-		NewDefaultRequestSigner(a.Certificate(), a.Chain(), a.Signer()),
+		NewDefaultRequestSigner(a.Certificate(), a.ChainWithAuthority(), a.Signer()),
 		a.leafMaker(groupKey))
 }
 

diff --git a/pki/authority_test.go b/pki/authority_test.go
@@ -76,6 +76,21 @@ func (a *AuthoritySuite) TestLeafRequest(c *gc.C) {
 	c.Assert(leaf.Certificate().IPAddresses, jc.DeepEquals, ipAddresses)
 }
 
+func (a *AuthoritySuite) TestLeafRequestChain(c *gc.C) {
+	authority, err := pki.NewDefaultAuthority(a.ca, a.signer)
+	c.Assert(err, jc.ErrorIsNil)
+	dnsNames := []string{"test.juju.is"}
+	ipAddresses := []net.IP{net.ParseIP("fe80:abcd::1")}
+	leaf, err := authority.LeafRequestForGroup("testgroup").
+		AddDNSNames(dnsNames...).
+		AddIPAddresses(ipAddresses...).
+		Commit()
+
+	chain := leaf.Chain()
+	c.Assert(len(chain), gc.Equals, 1)
+	c.Assert(chain[0], jc.DeepEquals, authority.Certificate())
+}
+
 func (a *AuthoritySuite) TestLeafFromPem(c *gc.C) {
 	authority, err := pki.NewDefaultAuthority(a.ca, a.signer)
 	c.Assert(err, jc.ErrorIsNil)

diff --git a/pki/tls/sni.go b/pki/tls/sni.go
@@ -21,14 +21,28 @@ type Logger interface {
 func AuthoritySNITLSGetter(authority pki.Authority, logger Logger) func(*tls.ClientHelloInfo) (*tls.Certificate, error) {
 	return func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
 		logger.Debugf("received tls client hello for server name %s", hello.ServerName)
+
 		var cert *tls.Certificate
-		authority.LeafRange(func(leaf pki.Leaf) bool {
-			if err := hello.SupportsCertificate(leaf.TLSCertificate()); err == nil {
-				cert = leaf.TLSCertificate()
-				return false
+
+		// NOTE: This was added in response to bug lp:1921557. If we get an
+		// empty server name here we assume the the connection is being made
+		// with an ip address as the host.
+		if hello.ServerName == "" {
+			logger.Debugf("tls client hello server name is empty. Attempting to provide ip address certificate")
+			leaf, err := authority.LeafForGroup(pki.ControllerIPLeafGroup)
+			if err != nil && !errors.IsNotFound(err) {
+				return nil, errors.Annotate(err, "fetching ip address certificate")
 			}
-			return true
-		})
+			cert = leaf.TLSCertificate()
+		} else {
+			authority.LeafRange(func(leaf pki.Leaf) bool {
+				if err := hello.SupportsCertificate(leaf.TLSCertificate()); err == nil {
+					cert = leaf.TLSCertificate()
+					return false
+				}
+				return true
+			})
+		}
 
 		if cert == nil {
 			logger.Debugf("no matching certificate found for tls client hello %s, using default certificate", hello.ServerName)

diff --git a/pki/tls/sni_test.go b/pki/tls/sni_test.go
@@ -89,6 +89,12 @@ func (s *SNISuite) TestAuthorityTLSGetter(c *gc.C) {
 			Group:         pki.DefaultLeafGroup,
 			ServerName:    "doesnotexist.juju.example",
 		},
+		{
+			// Regression test for LP1921557
+			ExpectedGroup: pki.ControllerIPLeafGroup,
+			Group:         pki.ControllerIPLeafGroup,
+			ServerName:    "",
+		},
 	}
 
 	for _, test := range tests {