Skip to content

Commit

Permalink
feat: address PR changes
Browse files Browse the repository at this point in the history 
- Don't leak api types outside of the jaas client.
- Cleanup tests
  • Loading branch information
@kian99
kian99 committed Aug 7, 2024
1 parent fc07a6a commit 36a5d48
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 48 deletions.
4 changes: 1 addition & 3 deletions internal/juju/common_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ type JujuSuite struct {
mockSharedClient *MockSharedClient
}

func (s *JujuSuite) setupMocks(t *testing.T, modelName *string) *gomock.Controller {
s.testModelName = modelName

func (s *JujuSuite) setupMocks(t *testing.T) *gomock.Controller {
ctlr := gomock.NewController(t)

s.mockConnection = NewMockConnection(ctlr)
Expand Down
42 changes: 34 additions & 8 deletions internal/juju/jaas.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ package juju
import (
"github.com/canonical/jimm-go-sdk/v3/api"
"github.com/canonical/jimm-go-sdk/v3/api/params"
jujuapi "github.com/juju/juju/api"

"github.com/juju/errors"
jujuapi "github.com/juju/juju/api"
)

type jaasClient struct {
Expand All @@ -25,36 +24,63 @@ func newJaasClient(sc SharedClient) *jaasClient {
}
}

// JaasTuple represents a tuple object of used by JAAS for permissions management.
type JaasTuple struct {
// Object represents the source side of the relation.
Object string
// Relation represents the level of access
Relation string
// Target represents the resource that you want `object` to have access to.
Target string
}

func toAPITuples(tuples []JaasTuple) []params.RelationshipTuple {
out := make([]params.RelationshipTuple, 0, len(tuples))
for _, tuple := range tuples {
out = append(out, toAPITuple(tuple))
}
return out
}

func toAPITuple(tuple JaasTuple) params.RelationshipTuple {
return params.RelationshipTuple{
Object: tuple.Object,
Relation: tuple.Relation,
TargetObject: tuple.Target,
}
}

// AddRelations attempts to create the provided slice of relationship tuples.
func (jc *jaasClient) AddRelations(tuples []params.RelationshipTuple) error {
// The caller is expected to populate the slice so that `len(tuples) > 0`.
func (jc *jaasClient) AddRelations(tuples []JaasTuple) error {
conn, err := jc.GetConnection(nil)
if err != nil {
return err
}
defer func() { _ = conn.Close() }()
cl := jc.getJaasApiClient(conn)
req := params.AddRelationRequest{
Tuples: tuples,
Tuples: toAPITuples(tuples),
}
return cl.AddRelation(&req)
}

// DeleteRelations attempts to delete the provided slice of relationship tuples.
func (jc *jaasClient) DeleteRelations(tuples []params.RelationshipTuple) error {
func (jc *jaasClient) DeleteRelations(tuples []JaasTuple) error {
conn, err := jc.GetConnection(nil)
if err != nil {
return err
}
defer func() { _ = conn.Close() }()
cl := jc.getJaasApiClient(conn)
req := params.RemoveRelationRequest{
Tuples: tuples,
Tuples: toAPITuples(tuples),
}
return cl.RemoveRelation(&req)
}

// ReadRelations attempts to read relations that match the criteria defined by `tuple`.
func (jc *jaasClient) ReadRelations(tuple *params.RelationshipTuple) ([]params.RelationshipTuple, error) {
func (jc *jaasClient) ReadRelations(tuple *JaasTuple) ([]params.RelationshipTuple, error) {
if tuple == nil {
return nil, errors.New("add relation request nil")
}
Expand All @@ -67,7 +93,7 @@ func (jc *jaasClient) ReadRelations(tuple *params.RelationshipTuple) ([]params.R

client := jc.getJaasApiClient(conn)
relations := make([]params.RelationshipTuple, 0)
req := &params.ListRelationshipTuplesRequest{Tuple: *tuple}
req := &params.ListRelationshipTuplesRequest{Tuple: toAPITuple(*tuple)}
for {
resp, err := client.ListRelationshipTuples(req)
if err != nil {
Expand Down
38 changes: 18 additions & 20 deletions internal/juju/jaas_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ type JaasSuite struct {
mockJaasClient *MockJaasAPIClient
}

func (s *JaasSuite) SetupTest() {}

func (s *JaasSuite) setupMocks(t *testing.T) *gomock.Controller {
ctlr := s.JujuSuite.setupMocks(t, nil)
ctlr := s.JujuSuite.setupMocks(t)
s.mockJaasClient = NewMockJaasAPIClient(ctlr)

return ctlr
Expand All @@ -41,17 +39,17 @@ func (s *JaasSuite) TestAddRelations() {
ctlr := s.setupMocks(s.T())
defer ctlr.Finish()

tuples := []params.RelationshipTuple{
{Object: "object-1", Relation: "relation", TargetObject: "target-1"},
{Object: "object-2", Relation: "relation", TargetObject: "target-2"},
tuples := []JaasTuple{
{Object: "object-1", Relation: "relation", Target: "target-1"},
{Object: "object-2", Relation: "relation", Target: "target-2"},
}
req := params.AddRelationRequest{
Tuples: tuples,
Tuples: toAPITuples(tuples),
}

s.mockJaasClient.EXPECT().AddRelation(
&req,
).Return(nil).Times(1)
).Return(nil)

client := s.getJaasClient()
err := client.AddRelations(tuples)
Expand All @@ -62,17 +60,17 @@ func (s *JaasSuite) TestDeleteRelations() {
ctlr := s.setupMocks(s.T())
defer ctlr.Finish()

tuples := []params.RelationshipTuple{
{Object: "object-1", Relation: "relation", TargetObject: "target-1"},
{Object: "object-2", Relation: "relation", TargetObject: "target-2"},
tuples := []JaasTuple{
{Object: "object-1", Relation: "relation", Target: "target-1"},
{Object: "object-2", Relation: "relation", Target: "target-2"},
}
req := params.RemoveRelationRequest{
Tuples: tuples,
Tuples: toAPITuples(tuples),
}

s.mockJaasClient.EXPECT().RemoveRelation(
&req,
).Return(nil).Times(1)
).Return(nil)

client := s.getJaasClient()
err := client.DeleteRelations(tuples)
Expand All @@ -83,25 +81,25 @@ func (s *JaasSuite) TestReadRelations() {
ctlr := s.setupMocks(s.T())
defer ctlr.Finish()

tuple := params.RelationshipTuple{Object: "object-1", Relation: "relation", TargetObject: "target-1"}
tuple := JaasTuple{Object: "object-1", Relation: "relation", Target: "target-1"}
// 1st request/response has no token in the request and a token in the response indicating another page is available.
req := &params.ListRelationshipTuplesRequest{Tuple: tuple}
req := &params.ListRelationshipTuplesRequest{Tuple: toAPITuple(tuple)}
respWithToken := &params.ListRelationshipTuplesResponse{
Tuples: []params.RelationshipTuple{tuple},
Tuples: []params.RelationshipTuple{toAPITuple(tuple)},
ContinuationToken: "token",
}
s.mockJaasClient.EXPECT().ListRelationshipTuples(
req,
).Return(respWithToken, nil).Times(1)
).Return(respWithToken, nil)
// 2nd request/response has the previous token in the request and no token in the response, indicating all pages have been consumed.
reqWithToken := &params.ListRelationshipTuplesRequest{Tuple: tuple, ContinuationToken: "token"}
reqWithToken := &params.ListRelationshipTuplesRequest{Tuple: toAPITuple(tuple), ContinuationToken: "token"}
respWithoutToken := &params.ListRelationshipTuplesResponse{
Tuples: []params.RelationshipTuple{tuple},
Tuples: []params.RelationshipTuple{toAPITuple(tuple)},
ContinuationToken: "",
}
s.mockJaasClient.EXPECT().ListRelationshipTuples(
reqWithToken,
).Return(respWithoutToken, nil).Times(1)
).Return(respWithoutToken, nil)

client := s.getJaasClient()
relations, err := client.ReadRelations(&tuple)
Expand Down
32 changes: 15 additions & 17 deletions internal/juju/secrets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,15 @@ type SecretSuite struct {
suite.Suite
JujuSuite

testModelName string

mockSecretClient *MockSecretAPIClient
}

func (s *SecretSuite) SetupTest() {}
func (s *SecretSuite) SetupSuite() {
s.testModelName = strPtr("test-secret-model")
}

func (s *SecretSuite) setupMocks(t *testing.T) *gomock.Controller {
s.testModelName = "test-secret-model"

ctlr := s.JujuSuite.setupMocks(t, &s.testModelName)
ctlr := s.JujuSuite.setupMocks(t)
s.mockSecretClient = NewMockSecretAPIClient(ctlr)

return ctlr
Expand Down Expand Up @@ -60,7 +58,7 @@ func (s *SecretSuite) TestCreateSecret() {

client := s.getSecretsClient()
output, err := client.CreateSecret(&CreateSecretInput{
ModelName: s.testModelName,
ModelName: *s.testModelName,
Name: "test-secret",
Value: decodedValue,
Info: "test info",
Expand All @@ -86,7 +84,7 @@ func (s *SecretSuite) TestCreateSecretError() {

client := s.getSecretsClient()
output, err := client.CreateSecret(&CreateSecretInput{
ModelName: s.testModelName,
ModelName: *s.testModelName,
Name: "test-secret",
Value: decodedValue,
Info: "test info",
Expand Down Expand Up @@ -133,7 +131,7 @@ func (s *SecretSuite) TestReadSecret() {
client := s.getSecretsClient()
output, err := client.ReadSecret(&ReadSecretInput{
SecretId: secretId,
ModelName: s.testModelName,
ModelName: *s.testModelName,
Name: &secretName,
Revision: &secretRevision,
})
Expand Down Expand Up @@ -165,7 +163,7 @@ func (s *SecretSuite) TestReadSecretError() {
client := s.getSecretsClient()
output, err := client.ReadSecret(&ReadSecretInput{
SecretId: secretId,
ModelName: s.testModelName,
ModelName: *s.testModelName,
})
s.Require().Error(err)

Expand Down Expand Up @@ -195,7 +193,7 @@ func (s *SecretSuite) TestUpdateSecretWithRenaming() {
client := s.getSecretsClient()
err = client.UpdateSecret(&UpdateSecretInput{
SecretId: secretId,
ModelName: s.testModelName,
ModelName: *s.testModelName,
Name: &newSecretName,
Value: &decodedValue,
AutoPrune: &autoPrune,
Expand Down Expand Up @@ -224,7 +222,7 @@ func (s *SecretSuite) TestUpdateSecretWithRenaming() {
// read secret and check if value is updated
output, err := client.ReadSecret(&ReadSecretInput{
SecretId: secretId,
ModelName: s.testModelName,
ModelName: *s.testModelName,
})
s.Require().NoError(err)

Expand Down Expand Up @@ -252,7 +250,7 @@ func (s *SecretSuite) TestUpdateSecret() {
client := s.getSecretsClient()
err = client.UpdateSecret(&UpdateSecretInput{
SecretId: secretId,
ModelName: s.testModelName,
ModelName: *s.testModelName,
Value: &decodedValue,
AutoPrune: &autoPrune,
Info: &secretInfo,
Expand Down Expand Up @@ -281,7 +279,7 @@ func (s *SecretSuite) TestUpdateSecret() {
// read secret and check if secret info is updated
output, err := client.ReadSecret(&ReadSecretInput{
SecretId: secretId,
ModelName: s.testModelName,
ModelName: *s.testModelName,
})
s.Require().NoError(err)

Expand All @@ -302,7 +300,7 @@ func (s *SecretSuite) TestDeleteSecret() {
client := s.getSecretsClient()
err = client.DeleteSecret(&DeleteSecretInput{
SecretId: secretId,
ModelName: s.testModelName,
ModelName: *s.testModelName,
})
s.Assert().NoError(err)
}
Expand All @@ -323,14 +321,14 @@ func (s *SecretSuite) TestUpdateAccessSecret() {
client := s.getSecretsClient()
err = client.UpdateAccessSecret(&GrantRevokeAccessSecretInput{
SecretId: secretId,
ModelName: s.testModelName,
ModelName: *s.testModelName,
Applications: applications,
}, GrantAccess)
s.Require().NoError(err)

err = client.UpdateAccessSecret(&GrantRevokeAccessSecretInput{
SecretId: secretId,
ModelName: s.testModelName,
ModelName: *s.testModelName,
Applications: applications,
}, RevokeAccess)
s.Require().NoError(err)
Expand Down

0 comments on commit 36a5d48

Please sign in to comment.