Skip to content

Commit

Permalink
chore: move model proxy to separate pkg (#1521)
Browse files Browse the repository at this point in the history
* chore: move model proxy to separate pkg

The model proxy is currently housed within the rpc package.
Originally I did this because the model proxy relied on the Juju RPC message definition declared in the rpc package.
But this  is likely not the right place for the model proxy as it has nothing to do with the dialer logic in the rpc package.
Another supporting argument that the model proxy was in the wrong package is that the jimm package relies on the rpc package for dialing logic, but does not rely on the model proxy at all.
And in fact, instead, the model proxy indirectly relies on the jimm package for business logic.

* chore: remove unused isRequest func
  • Loading branch information
kian99 authored Jan 15, 2025
1 parent 6f1ac86 commit 217bd0f
Show file tree
Hide file tree
Showing 15 changed files with 509 additions and 425 deletions.
4 changes: 2 additions & 2 deletions internal/jujuapi/streamproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/canonical/jimm/v3/internal/errors"
"github.com/canonical/jimm/v3/internal/jimmhttp"
"github.com/canonical/jimm/v3/internal/openfga"
jimmRPC "github.com/canonical/jimm/v3/internal/rpc"
"github.com/canonical/jimm/v3/internal/streamproxy"
)

// A streamProxier serves the the /log endpoint by proxying
Expand Down Expand Up @@ -103,7 +103,7 @@ func (s streamProxier) ServeWS(ctx context.Context, clientConn *websocket.Conn)
return
}

jimmRPC.ProxyStreams(ctx, clientConn, controllerStream)
streamproxy.ProxyStreams(ctx, clientConn, controllerStream)
}

func checkPermission(ctx context.Context, path string, u *openfga.User, mt names.ModelTag) (bool, error) {
Expand Down
17 changes: 9 additions & 8 deletions internal/jujuapi/websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/canonical/jimm/v3/internal/jimm/jujuauth"
"github.com/canonical/jimm/v3/internal/jimmhttp"
jimmRPC "github.com/canonical/jimm/v3/internal/rpc"
"github.com/canonical/jimm/v3/internal/rpcproxy"
)

const (
Expand Down Expand Up @@ -177,30 +178,30 @@ func (s apiProxier) ServeWS(ctx context.Context, clientConn *websocket.Conn) {
connectionFunc := controllerConnectionFunc(s, &jwtGenerator)
zapctx.Debug(ctx, "Starting proxier")
auditLogger := s.jimm.AuditLogManager().AddAuditLogEntry
proxyHelpers := jimmRPC.ProxyHelpers{
proxyHelpers := rpcproxy.ProxyHelpers{
ConnClient: clientConn,
TokenGen: &jwtGenerator,
ConnectController: connectionFunc,
AuditLog: auditLogger,
LoginService: s.jimm.LoginManager(),
AuthenticatedIdentityID: auth.SessionIdentityFromContext(ctx),
}
if err := jimmRPC.ProxySockets(ctx, proxyHelpers); err != nil {
if err := rpcproxy.ProxySockets(ctx, proxyHelpers); err != nil {
zapctx.Error(ctx, "failed to start jimm model proxy", zap.Error(err))
}
}

// controllerConnectionFunc returns a function that will be used to
// connect to a controller when a client makes a request.
func controllerConnectionFunc(s apiProxier, jwtGenerator *jujuauth.TokenGenerator) func(context.Context) (jimmRPC.WebsocketConnectionWithMetadata, error) {
return func(ctx context.Context) (jimmRPC.WebsocketConnectionWithMetadata, error) {
func controllerConnectionFunc(s apiProxier, jwtGenerator *jujuauth.TokenGenerator) func(context.Context) (rpcproxy.WebsocketConnectionWithMetadata, error) {
return func(ctx context.Context) (rpcproxy.WebsocketConnectionWithMetadata, error) {
const op = errors.Op("proxy.controllerConnectionFunc")
path := jimmhttp.PathElementFromContext(ctx, "path")
zapctx.Debug(ctx, "grabbing model info from path", zap.String("path", path))
uuid, finalPath, err := modelInfoFromPath(path)
if err != nil {
zapctx.Error(ctx, "error parsing path", zap.Error(err))
return jimmRPC.WebsocketConnectionWithMetadata{}, errors.E(op, err)
return rpcproxy.WebsocketConnectionWithMetadata{}, errors.E(op, err)
}
m := dbmodel.Model{
UUID: sql.NullString{
Expand All @@ -210,18 +211,18 @@ func controllerConnectionFunc(s apiProxier, jwtGenerator *jujuauth.TokenGenerato
}
if err := s.jimm.Database.GetModel(context.Background(), &m); err != nil {
zapctx.Error(ctx, "failed to find model", zap.String("uuid", uuid), zap.Error(err))
return jimmRPC.WebsocketConnectionWithMetadata{}, errors.E(err, errors.CodeNotFound)
return rpcproxy.WebsocketConnectionWithMetadata{}, errors.E(err, errors.CodeNotFound)
}
jwtGenerator.SetTags(m.ResourceTag(), m.Controller.ResourceTag())
mt := m.ResourceTag()
zapctx.Debug(ctx, "Dialing Controller", zap.String("path", path))
controllerConn, err := jimmRPC.Dial(ctx, &m.Controller, mt, finalPath, nil)
if err != nil {
zapctx.Error(ctx, "cannot dial controller", zap.String("controller", m.Controller.Name), zap.Error(err))
return jimmRPC.WebsocketConnectionWithMetadata{}, err
return rpcproxy.WebsocketConnectionWithMetadata{}, err
}
fullModelName := m.Controller.Name + "/" + m.Name
return jimmRPC.WebsocketConnectionWithMetadata{
return rpcproxy.WebsocketConnectionWithMetadata{
Conn: controllerConn,
ControllerUUID: m.Controller.UUID,
ModelName: fullModelName,
Expand Down
12 changes: 1 addition & 11 deletions internal/rpc/client.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 Canonical.
// Copyright 2025 Canonical.

package rpc

Expand Down Expand Up @@ -203,16 +203,6 @@ func (c *Client) Call(ctx context.Context, facade string, version int, id, metho

select {
case <-ch:
permissionsRequired, err := checkPermissionsRequired(ctx, *respMsg)
if err != nil {
return err
}
if permissionsRequired != nil {
return &Error{
Code: PermissionCheckRequiredErrorCode,
Info: permissionsRequired,
}
}
if (*respMsg).Error != "" {
return &Error{
Message: (*respMsg).Error,
Expand Down
Loading

0 comments on commit 217bd0f

Please sign in to comment.