Merge pull request juju#14615 from ycliuhw/fix/lp-1989160

juju#14615 Currently, only model super users can SSH/SCP for k8s models because the current implementation uses the model credential to call the exec API. This PR ensures the model operator creates a set of RBAC resources for exec purposes and changes the SSH/SCP command to use the new service account token. This allows any users having a model admin role to use SSH/SCP for k8s models. - [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/develop/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 ```console $ juju bootstrap microk8s k1 $ juju add-model t1 $ mkubectl -ncontroller-k1 get sa/model-exec NAME SECRETS AGE model-exec 1 3m55s $ mkubectl -nt1 get sa/model-exec NAME SECRETS AGE model-exec 1 3m51s $ juju deploy snappass-test $ juju ssh -m k1:controller 0 # env | grep HOSTNAME HOSTNAME=controller-0 $ juju ssh -m k1:t1 snappass-test/0 # env | grep HOSTNAME HOSTNAME=snappass-test-0 $ juju add-user u-read $ juju add-user u-write $ juju add-user u-admin $ juju grant u-write write t1 $ juju grant u-read read t1 $ juju grant u-admin admin t1 $ juju logout --force Logged out. You are no longer logged into any controllers. $ juju login -u u-read -c k1 please enter password for u-read on k1: Welcome, u-read. You are now logged into "k1". Current model set to "admin/t1". $ juju ssh -m k1:admin/t1 snappass-test/0 ERROR permission denied (unauthorized access) $ juju logout --force Logged out. You are no longer logged into any controllers. $ juju login -u u-write -c k1 please enter password for u-write on k1: Welcome, u-write. You are now logged into "k1". Current model set to "admin/t1". $ juju ssh -m k1:admin/t1 snappass-test/0 ERROR permission denied (unauthorized access) $ juju logout --force Logged out. You are no longer logged into any controllers. $ juju login -u u-admin -c k1 please enter password for u-admin on k1: Welcome, u-admin. You are now logged into "k1". Current model set to "admin/t1". $ juju ssh -m k1:admin/t1 snappass-test/0 # env | grep HOSTNAME HOSTNAME=snappass-test-0 ``` ## Documentation changes No ## Bug reference https://bugs.launchpad.net/juju/+bug/1989160
bnhassin · Sep 20, 2022 · 231abcd · 231abcd
2 parents 0537910 + 1c9aae3
commit 231abcd
Show file tree

Hide file tree

Showing 27 changed files with 1,581 additions and 131 deletions.
diff --git a/api/client/sshclient/facade.go b/api/client/sshclient/facade.go
@@ -8,6 +8,9 @@ import (
 	"github.com/juju/names/v4"
 
 	"github.com/juju/juju/api/base"
+	apiservererrors "github.com/juju/juju/apiserver/errors"
+	"github.com/juju/juju/cloud"
+	"github.com/juju/juju/environs/cloudspec"
 	"github.com/juju/juju/rpc/params"
 )
 
@@ -150,3 +153,46 @@ func targetToTag(target string) (names.Tag, error) {
 func countError(count int) error {
 	return errors.Errorf("expected 1 result, got %d", count)
 }
+
+// ModelCredentialForSSH returns a cloud spec for ssh purpose.
+// This facade call is only used for k8s model.
+func (facade *Facade) ModelCredentialForSSH() (cloudspec.CloudSpec, error) {
+	var result params.CloudSpecResult
+
+	err := facade.caller.FacadeCall("ModelCredentialForSSH", nil, &result)
+	if err != nil {
+		return cloudspec.CloudSpec{}, err
+	}
+	if result.Error != nil {
+		err := apiservererrors.RestoreError(result.Error)
+		return cloudspec.CloudSpec{}, err
+	}
+	pSpec := result.Result
+	if pSpec == nil {
+		return cloudspec.CloudSpec{}, errors.NotValidf("empty value")
+	}
+	var credential *cloud.Credential
+	if pSpec.Credential != nil {
+		credentialValue := cloud.NewCredential(
+			cloud.AuthType(pSpec.Credential.AuthType),
+			pSpec.Credential.Attributes,
+		)
+		credential = &credentialValue
+	}
+	spec := cloudspec.CloudSpec{
+		Type:              pSpec.Type,
+		Name:              pSpec.Name,
+		Region:            pSpec.Region,
+		Endpoint:          pSpec.Endpoint,
+		IdentityEndpoint:  pSpec.IdentityEndpoint,
+		StorageEndpoint:   pSpec.StorageEndpoint,
+		CACertificates:    pSpec.CACertificates,
+		SkipTLSVerify:     pSpec.SkipTLSVerify,
+		Credential:        credential,
+		IsControllerCloud: pSpec.IsControllerCloud,
+	}
+	if err := spec.Validate(); err != nil {
+		return cloudspec.CloudSpec{}, errors.Annotatef(err, "cannot validate CloudSpec %q", spec.Name)
+	}
+	return spec, nil
+}
diff --git a/api/client/sshclient/facade_test.go b/api/client/sshclient/facade_test.go
@@ -13,7 +13,11 @@ import (
 	apitesting "github.com/juju/juju/api/base/testing"
 	"github.com/juju/juju/api/client/sshclient"
 	apiservererrors "github.com/juju/juju/apiserver/errors"
+	k8scloud "github.com/juju/juju/caas/kubernetes/cloud"
+	"github.com/juju/juju/cloud"
+	environscloudspec "github.com/juju/juju/environs/cloudspec"
 	"github.com/juju/juju/rpc/params"
+	"github.com/juju/juju/testing"
 )
 
 type FacadeSuite struct {
@@ -296,3 +300,59 @@ func (s *FacadeSuite) TestLeader(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(obtainedUnit, gc.Equals, "ubuntu/42")
 }
+
+func (s *FacadeSuite) TestModelCredentialForSSH(c *gc.C) {
+	apiCaller := apitesting.APICallerFunc(func(objType string, version int, id, request string, arg, result interface{}) error {
+		c.Check(objType, gc.Equals, "SSHClient")
+		c.Check(request, gc.Equals, "ModelCredentialForSSH")
+		c.Assert(arg, gc.IsNil)
+		c.Assert(result, gc.FitsTypeOf, &params.CloudSpecResult{})
+
+		*(result.(*params.CloudSpecResult)) = params.CloudSpecResult{
+			Result: &params.CloudSpec{
+				Type:             "type",
+				Name:             "name",
+				Region:           "region",
+				Endpoint:         "endpoint",
+				IdentityEndpoint: "identity-endpoint",
+				StorageEndpoint:  "storage-endpoint",
+				Credential: &params.CloudCredential{
+					AuthType: "auth-type",
+					Attributes: map[string]string{
+						k8scloud.CredAttrUsername: "",
+						k8scloud.CredAttrPassword: "",
+						k8scloud.CredAttrToken:    "token",
+					},
+				},
+				CACertificates: []string{testing.CACert},
+				SkipTLSVerify:  true,
+			},
+		}
+		return nil
+	})
+
+	facade := sshclient.NewFacade(apiCaller)
+	spec, err := facade.ModelCredentialForSSH()
+	c.Assert(err, jc.ErrorIsNil)
+
+	credential := cloud.NewCredential(
+		"auth-type",
+		map[string]string{
+			k8scloud.CredAttrUsername: "",
+			k8scloud.CredAttrPassword: "",
+			k8scloud.CredAttrToken:    "token",
+		},
+	)
+	cloudSpec := environscloudspec.CloudSpec{
+		Type:             "type",
+		Name:             "name",
+		Region:           "region",
+		Endpoint:         "endpoint",
+		IdentityEndpoint: "identity-endpoint",
+		StorageEndpoint:  "storage-endpoint",
+		Credential:       &credential,
+		CACertificates:   []string{testing.CACert},
+		SkipTLSVerify:    true,
+	}
+	c.Assert(spec, gc.DeepEquals, cloudSpec)
+}
diff --git a/api/facadeversions.go b/api/facadeversions.go
@@ -109,7 +109,7 @@ var facadeVersions = map[string]int{
 	"SecretsManager":               1,
 	"Singular":                     2,
 	"Spaces":                       6,
-	"SSHClient":                    3,
+	"SSHClient":                    4,
 	"StatusHistory":                2,
 	"Storage":                      6,
 	"StorageProvisioner":           4,

diff --git a/apiserver/facades/client/sshclient/facade.go b/apiserver/facades/client/sshclient/facade.go
@@ -4,39 +4,61 @@
 package sshclient
 
 import (
+	stdcontext "context"
 	"sort"
 
 	"github.com/juju/errors"
 	"github.com/juju/names/v4"
 
 	apiservererrors "github.com/juju/juju/apiserver/errors"
 	"github.com/juju/juju/apiserver/facade"
+	k8scloud "github.com/juju/juju/caas/kubernetes/cloud"
+	k8sprovider "github.com/juju/juju/caas/kubernetes/provider"
 	"github.com/juju/juju/core/leadership"
 	"github.com/juju/juju/core/network"
 	"github.com/juju/juju/core/permission"
+	"github.com/juju/juju/environs"
+	environscloudspec "github.com/juju/juju/environs/cloudspec"
 	"github.com/juju/juju/environs/context"
 	"github.com/juju/juju/rpc/params"
+	"github.com/juju/juju/state"
 )
 
+type newCaasBrokerFunc func(_ stdcontext.Context, args environs.OpenParams) (Broker, error)
+
 // Facade implements the API required by the sshclient worker.
 type Facade struct {
 	backend     Backend
 	authorizer  facade.Authorizer
 	callContext context.ProviderCallContext
 
 	leadershipReader leadership.Reader
+	getBroker        newCaasBrokerFunc
 }
 
-type FacadeV2 struct {
+type FacadeV3 struct {
 	*Facade
 }
 
-func internalFacade(backend Backend, leadershipReader leadership.Reader, auth facade.Authorizer, callCtx context.ProviderCallContext) (*Facade, error) {
+type FacadeV2 struct {
+	*FacadeV3
+}
+
+func internalFacade(
+	backend Backend, leadershipReader leadership.Reader, auth facade.Authorizer, callCtx context.ProviderCallContext,
+	getBroker newCaasBrokerFunc,
+) (*Facade, error) {
 	if !auth.AuthClient() {
 		return nil, apiservererrors.ErrPerm
 	}
 
-	return &Facade{backend: backend, authorizer: auth, callContext: callCtx, leadershipReader: leadershipReader}, nil
+	return &Facade{
+		backend:          backend,
+		authorizer:       auth,
+		callContext:      callCtx,
+		leadershipReader: leadershipReader,
+		getBroker:        getBroker,
+	}, nil
 }
 
 func (facade *Facade) checkIsModelAdmin() error {
@@ -228,3 +250,89 @@ func (facade *Facade) Leader(entity params.Entity) (params.StringResult, error)
 	}
 	return result, nil
 }
+
+// ModelCredentialForSSH did not exist prior to v4.
+func (*FacadeV3) ModelCredentialForSSH(_, _ struct{}) {}
+
+// ModelCredentialForSSH returns a cloud spec for ssh purpose.
+// This facade call is only used for k8s model.
+func (facade *Facade) ModelCredentialForSSH() (params.CloudSpecResult, error) {
+	var result params.CloudSpecResult
+
+	isModelAdmin, err := facade.authorizer.HasPermission(permission.AdminAccess, facade.backend.ModelTag())
+	if err != nil {
+		return result, errors.Trace(err)
+	}
+	isSuperUser, err := facade.authorizer.HasPermission(permission.SuperuserAccess, facade.backend.ControllerTag())
+	if err != nil {
+		return result, errors.Trace(err)
+	}
+	if !isModelAdmin && !isSuperUser {
+		return result, apiservererrors.ErrPerm
+	}
+
+	model, err := facade.backend.Model()
+	if err != nil {
+		result.Error = apiservererrors.ServerError(err)
+		return result, nil
+	}
+	if model.Type() != state.ModelTypeCAAS {
+		result.Error = apiservererrors.ServerError(errors.NotSupportedf("facade ModelCredentialForSSH for non %q model", state.ModelTypeCAAS))
+		return result, nil
+	}
+
+	spec, err := facade.backend.CloudSpec()
+	if err != nil {
+		result.Error = apiservererrors.ServerError(err)
+		return result, nil
+	}
+	if spec.Credential == nil {
+		result.Error = apiservererrors.ServerError(errors.NotValidf("cloud spec %q has empty credential", spec.Name))
+		return result, nil
+	}
+
+	token, err := facade.getExecSecretToken(spec, model)
+	if err != nil {
+		result.Error = apiservererrors.ServerError(err)
+		return result, nil
+	}
+
+	cred, err := k8scloud.UpdateCredentialWithToken(*spec.Credential, token)
+	if err != nil {
+		result.Error = apiservererrors.ServerError(err)
+		return result, nil
+	}
+	result.Result = &params.CloudSpec{
+		Type:             spec.Type,
+		Name:             spec.Name,
+		Region:           spec.Region,
+		Endpoint:         spec.Endpoint,
+		IdentityEndpoint: spec.IdentityEndpoint,
+		StorageEndpoint:  spec.StorageEndpoint,
+		Credential: &params.CloudCredential{
+			AuthType:   string(cred.AuthType()),
+			Attributes: cred.Attributes(),
+		},
+		CACertificates:    spec.CACertificates,
+		SkipTLSVerify:     spec.SkipTLSVerify,
+		IsControllerCloud: spec.IsControllerCloud,
+	}
+	return result, nil
+}
+
+func (facade *Facade) getExecSecretToken(cloudSpec environscloudspec.CloudSpec, model Model) (string, error) {
+	cfg, err := model.Config()
+	if err != nil {
+		return "", errors.Trace(err)
+	}
+
+	broker, err := facade.getBroker(stdcontext.TODO(), environs.OpenParams{
+		ControllerUUID: model.ControllerUUID(),
+		Cloud:          cloudSpec,
+		Config:         cfg,
+	})
+	if err != nil {
+		return "", errors.Annotate(err, "failed to open kubernetes client")
+	}
+	return broker.GetSecretToken(k8sprovider.ExecRBACResourceName)
+}