From b3646b1707289bbd891ceae2d610eb876b3b228d Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Tue, 6 Aug 2024 09:47:43 +0200 Subject: [PATCH 1/9] feat: add jaas client --- go.mod | 6 +-- go.sum | 8 ++-- internal/juju/client.go | 3 ++ internal/juju/interfaces.go | 7 ++++ internal/juju/jaas.go | 81 +++++++++++++++++++++++++++++++++++++ internal/juju/jaas_test.go | 28 +++++++++++++ 6 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 internal/juju/jaas.go create mode 100644 internal/juju/jaas_test.go diff --git a/go.mod b/go.mod index 2d71cafe..2796d3fe 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/juju/terraform-provider-juju -go 1.21 +go 1.22.3 require ( github.com/bflad/tfproviderlint v0.30.0 @@ -199,7 +199,7 @@ require ( go.abhg.dev/goldmark/frontmatter v0.2.0 // indirect golang.org/x/crypto v0.25.0 // indirect golang.org/x/exp v0.0.0-20231127185646-65229373498e // indirect - golang.org/x/mod v0.18.0 // indirect + golang.org/x/mod v0.19.0 // indirect golang.org/x/net v0.27.0 // indirect golang.org/x/oauth2 v0.21.0 // indirect golang.org/x/sync v0.7.0 // indirect @@ -207,7 +207,7 @@ require ( golang.org/x/term v0.22.0 // indirect golang.org/x/text v0.16.0 // indirect golang.org/x/time v0.5.0 // indirect - golang.org/x/tools v0.22.0 // indirect + golang.org/x/tools v0.23.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240227224415-6ceb2ff114de // indirect google.golang.org/grpc v1.63.2 // indirect diff --git a/go.sum b/go.sum index 8e730edf..bdad4a7b 100644 --- a/go.sum +++ b/go.sum @@ -685,8 +685,8 @@ golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzB golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= -golang.org/x/mod v0.18.0 h1:5+9lSbEzPSdWkH32vYPBwEpX8KwDbM52Ud9xBUvNlb0= -golang.org/x/mod v0.18.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/mod v0.19.0 h1:fEdghXQSo20giMthA7cd28ZC+jts4amQ3YMXiP5oMQ8= +golang.org/x/mod v0.19.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -762,8 +762,8 @@ golang.org/x/tools v0.0.0-20200505023115-26f46d2f7ef8/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.22.0 h1:gqSGLZqv+AI9lIQzniJ0nZDRG5GBPsSi+DRNHWNz6yA= -golang.org/x/tools v0.22.0/go.mod h1:aCwcsjqvq7Yqt6TNyX7QMU2enbQ/Gt0bo6krSeEri+c= +golang.org/x/tools v0.23.0 h1:SGsXPZ+2l4JsgaCKkx+FQ9YZ5XEtA1GZYuoDjenLjvg= +golang.org/x/tools v0.23.0/go.mod h1:pnu6ufv6vQkll6szChhK3C3L/ruaIv5eBeztNG8wtsI= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/juju/client.go b/internal/juju/client.go index 41d2d19a..97d7fe1b 100644 --- a/internal/juju/client.go +++ b/internal/juju/client.go @@ -49,6 +49,7 @@ type Client struct { SSHKeys sshKeysClient Users usersClient Secrets secretsClient + Jaas jaasClient isJAAS func() bool } @@ -57,6 +58,7 @@ type Client struct { // JAAS controllers offer additional functionality for permission management. func (c Client) IsJAAS() bool { return c.isJAAS() + } type jujuModel struct { @@ -107,6 +109,7 @@ func NewClient(ctx context.Context, config ControllerConfiguration) (*Client, er SSHKeys: *newSSHKeysClient(sc), Users: *newUsersClient(sc), Secrets: *newSecretsClient(sc), + Jaas: *newJaasClient(sc), isJAAS: func() bool { return sc.IsJAAS(defaultJAASCheck) }, }, nil } diff --git a/internal/juju/interfaces.go b/internal/juju/interfaces.go index 7f07ddce..7190bd87 100644 --- a/internal/juju/interfaces.go +++ b/internal/juju/interfaces.go @@ -4,6 +4,7 @@ package juju import ( + jaasparams "github.com/canonical/jimm-go-sdk/v3/api/params" "github.com/juju/charm/v12" "github.com/juju/juju/api" apiapplication "github.com/juju/juju/api/client/application" @@ -76,3 +77,9 @@ type SecretAPIClient interface { GrantSecret(uri *secrets.URI, name string, apps []string) ([]error, error) RevokeSecret(uri *secrets.URI, name string, apps []string) ([]error, error) } + +type JaasApiClient interface { + ListRelationshipTuples(req *jaasparams.ListRelationshipTuplesRequest) (*jaasparams.ListRelationshipTuplesResponse, error) + AddRelation(req *jaasparams.AddRelationRequest) error + RemoveRelation(req *jaasparams.RemoveRelationRequest) error +} diff --git a/internal/juju/jaas.go b/internal/juju/jaas.go new file mode 100644 index 00000000..ded39798 --- /dev/null +++ b/internal/juju/jaas.go @@ -0,0 +1,81 @@ +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" +) + +type jaasClient struct { + SharedClient + getJaasApiClient func(jujuapi.Connection) JaasApiClient +} + +func newJaasClient(sc SharedClient) *jaasClient { + return &jaasClient{ + SharedClient: sc, + getJaasApiClient: func(conn jujuapi.Connection) JaasApiClient { + return api.NewClient(conn) + }, + } +} + +func (jc *jaasClient) AddTuples(tuples []params.RelationshipTuple) error { + conn, err := jc.GetConnection(nil) + if err != nil { + return err + } + defer func() { _ = conn.Close() }() + cl := jc.getJaasApiClient(conn) + req := params.AddRelationRequest{ + Tuples: tuples, + } + return cl.AddRelation(&req) +} + +func (jc *jaasClient) DeleteTuples(tuples []params.RelationshipTuple) error { + conn, err := jc.GetConnection(nil) + if err != nil { + return err + } + defer func() { _ = conn.Close() }() + cl := jc.getJaasApiClient(conn) + req := params.RemoveRelationRequest{ + Tuples: tuples, + } + return cl.RemoveRelation(&req) +} + +func (jc *jaasClient) ReadRelation(tuple *params.RelationshipTuple) ([]params.RelationshipTuple, error) { + if tuple == nil { + return nil, errors.New("add relation request nil") + } + + conn, err := jc.GetConnection(nil) + if err != nil { + return nil, err + } + defer func() { _ = conn.Close() }() + + client := jc.getJaasApiClient(conn) + relations := make([]params.RelationshipTuple, 0) + req := ¶ms.ListRelationshipTuplesRequest{Tuple: *tuple} + for { + resp, err := client.ListRelationshipTuples(req) + if err != nil { + jc.Errorf(err, "call to ListRelationshipTuples failed") + return nil, err + } + if len(resp.Errors) > 0 { + jc.Errorf(err, "call to ListRelationshipTuples contained error(s)") + return nil, errors.New(resp.Errors[0]) + } + relations = append(relations, resp.Tuples...) + if resp.ContinuationToken == "" { + return relations, nil + } + req.ContinuationToken = resp.ContinuationToken + } +} diff --git a/internal/juju/jaas_test.go b/internal/juju/jaas_test.go new file mode 100644 index 00000000..da987c2b --- /dev/null +++ b/internal/juju/jaas_test.go @@ -0,0 +1,28 @@ +package juju + +import ( + "testing" + + "github.com/stretchr/testify/suite" + "go.uber.org/mock/gomock" +) + +type JaasSuite struct { + suite.Suite + JujuSuite + + testModelName string + + mockJaasClient *MockSecretAPIClient +} + +func (s *JaasSuite) SetupTest() {} + +func (s *JaasSuite) setupMocks(t *testing.T) *gomock.Controller { + s.testModelName = "test-secret-model" + + ctlr := s.JujuSuite.setupMocks(t) + s.mockJaasClient = NewMockSecretAPIClient(ctlr) + + return ctlr +} From 794680c9004b0a2229475a5c327017a918dd39a6 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Tue, 6 Aug 2024 11:14:17 +0200 Subject: [PATCH 2/9] feat: add mock client tests --- internal/juju/common_test.go | 8 +-- internal/juju/interfaces.go | 2 +- internal/juju/jaas.go | 13 +++-- internal/juju/jaas_test.go | 102 +++++++++++++++++++++++++++++++--- internal/juju/mock_test.go | 101 +++++++++++++++++++++++++++------ internal/juju/offers.go | 3 +- internal/juju/package_test.go | 2 +- internal/juju/secrets_test.go | 2 +- 8 files changed, 195 insertions(+), 38 deletions(-) diff --git a/internal/juju/common_test.go b/internal/juju/common_test.go index a02b54a4..c392ddb8 100644 --- a/internal/juju/common_test.go +++ b/internal/juju/common_test.go @@ -13,14 +13,14 @@ import ( type JujuSuite struct { suite.Suite - testModelName string + testModelName *string mockConnection *MockConnection mockSharedClient *MockSharedClient } -func (s *JujuSuite) setupMocks(t *testing.T) *gomock.Controller { - s.testModelName = "test-secret-model" +func (s *JujuSuite) setupMocks(t *testing.T, modelName *string) *gomock.Controller { + s.testModelName = modelName ctlr := gomock.NewController(t) @@ -35,7 +35,7 @@ func (s *JujuSuite) setupMocks(t *testing.T) *gomock.Controller { s.mockSharedClient.EXPECT().Errorf(gomock.Any(), gomock.Any()).Do(log).AnyTimes() s.mockSharedClient.EXPECT().Tracef(gomock.Any(), gomock.Any()).Do(log).AnyTimes() s.mockSharedClient.EXPECT().JujuLogger().Return(&jujuLoggerShim{}).AnyTimes() - s.mockSharedClient.EXPECT().GetConnection(&s.testModelName).Return(s.mockConnection, nil).AnyTimes() + s.mockSharedClient.EXPECT().GetConnection(s.testModelName).Return(s.mockConnection, nil).AnyTimes() return ctlr } diff --git a/internal/juju/interfaces.go b/internal/juju/interfaces.go index 7190bd87..be8460a4 100644 --- a/internal/juju/interfaces.go +++ b/internal/juju/interfaces.go @@ -78,7 +78,7 @@ type SecretAPIClient interface { RevokeSecret(uri *secrets.URI, name string, apps []string) ([]error, error) } -type JaasApiClient interface { +type JaasAPIClient interface { ListRelationshipTuples(req *jaasparams.ListRelationshipTuplesRequest) (*jaasparams.ListRelationshipTuplesResponse, error) AddRelation(req *jaasparams.AddRelationRequest) error RemoveRelation(req *jaasparams.RemoveRelationRequest) error diff --git a/internal/juju/jaas.go b/internal/juju/jaas.go index ded39798..5d1ef495 100644 --- a/internal/juju/jaas.go +++ b/internal/juju/jaas.go @@ -1,3 +1,6 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the Apache License, Version 2.0, see LICENCE file for details. + package juju import ( @@ -10,19 +13,19 @@ import ( type jaasClient struct { SharedClient - getJaasApiClient func(jujuapi.Connection) JaasApiClient + getJaasApiClient func(jujuapi.Connection) JaasAPIClient } func newJaasClient(sc SharedClient) *jaasClient { return &jaasClient{ SharedClient: sc, - getJaasApiClient: func(conn jujuapi.Connection) JaasApiClient { + getJaasApiClient: func(conn jujuapi.Connection) JaasAPIClient { return api.NewClient(conn) }, } } -func (jc *jaasClient) AddTuples(tuples []params.RelationshipTuple) error { +func (jc *jaasClient) AddRelations(tuples []params.RelationshipTuple) error { conn, err := jc.GetConnection(nil) if err != nil { return err @@ -35,7 +38,7 @@ func (jc *jaasClient) AddTuples(tuples []params.RelationshipTuple) error { return cl.AddRelation(&req) } -func (jc *jaasClient) DeleteTuples(tuples []params.RelationshipTuple) error { +func (jc *jaasClient) DeleteRelations(tuples []params.RelationshipTuple) error { conn, err := jc.GetConnection(nil) if err != nil { return err @@ -48,7 +51,7 @@ func (jc *jaasClient) DeleteTuples(tuples []params.RelationshipTuple) error { return cl.RemoveRelation(&req) } -func (jc *jaasClient) ReadRelation(tuple *params.RelationshipTuple) ([]params.RelationshipTuple, error) { +func (jc *jaasClient) ReadRelations(tuple *params.RelationshipTuple) ([]params.RelationshipTuple, error) { if tuple == nil { return nil, errors.New("add relation request nil") } diff --git a/internal/juju/jaas_test.go b/internal/juju/jaas_test.go index da987c2b..f0589a35 100644 --- a/internal/juju/jaas_test.go +++ b/internal/juju/jaas_test.go @@ -1,8 +1,13 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the Apache License, Version 2.0, see LICENCE file for details. + package juju import ( "testing" + "github.com/canonical/jimm-go-sdk/v3/api/params" + "github.com/juju/juju/api" "github.com/stretchr/testify/suite" "go.uber.org/mock/gomock" ) @@ -11,18 +16,101 @@ type JaasSuite struct { suite.Suite JujuSuite - testModelName string - - mockJaasClient *MockSecretAPIClient + mockJaasClient *MockJaasAPIClient } func (s *JaasSuite) SetupTest() {} func (s *JaasSuite) setupMocks(t *testing.T) *gomock.Controller { - s.testModelName = "test-secret-model" - - ctlr := s.JujuSuite.setupMocks(t) - s.mockJaasClient = NewMockSecretAPIClient(ctlr) + ctlr := s.JujuSuite.setupMocks(t, nil) + s.mockJaasClient = NewMockJaasAPIClient(ctlr) return ctlr } + +func (s *JaasSuite) getJaasClient() jaasClient { + return jaasClient{ + SharedClient: s.JujuSuite.mockSharedClient, + getJaasApiClient: func(connection api.Connection) JaasAPIClient { + return s.mockJaasClient + }, + } +} + +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"}, + } + req := params.AddRelationRequest{ + Tuples: tuples, + } + + s.mockJaasClient.EXPECT().AddRelation( + &req, + ).Return(nil).Times(1) + + client := s.getJaasClient() + err := client.AddRelations(tuples) + s.Require().NoError(err) +} + +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"}, + } + req := params.RemoveRelationRequest{ + Tuples: tuples, + } + + s.mockJaasClient.EXPECT().RemoveRelation( + &req, + ).Return(nil).Times(1) + + client := s.getJaasClient() + err := client.DeleteRelations(tuples) + s.Require().NoError(err) +} + +func (s *JaasSuite) TestReadRelations() { + ctlr := s.setupMocks(s.T()) + defer ctlr.Finish() + + tuple := params.RelationshipTuple{Object: "object-1", Relation: "relation", TargetObject: "target-1"} + // 1st request/response has no token in the request and a token in the response indicating another page is available. + req := ¶ms.ListRelationshipTuplesRequest{Tuple: tuple} + respWithToken := ¶ms.ListRelationshipTuplesResponse{ + Tuples: []params.RelationshipTuple{tuple}, + ContinuationToken: "token", + } + s.mockJaasClient.EXPECT().ListRelationshipTuples( + req, + ).Return(respWithToken, nil).Times(1) + // 2nd request/response has the previous token in the request and no token in the response, indicating all pages have been consumed. + reqWithToken := ¶ms.ListRelationshipTuplesRequest{Tuple: tuple, ContinuationToken: "token"} + respWithoutToken := ¶ms.ListRelationshipTuplesResponse{ + Tuples: []params.RelationshipTuple{tuple}, + ContinuationToken: "", + } + s.mockJaasClient.EXPECT().ListRelationshipTuples( + reqWithToken, + ).Return(respWithoutToken, nil).Times(1) + + client := s.getJaasClient() + relations, err := client.ReadRelations(&tuple) + s.Require().NoError(err) + s.Require().Len(relations, 2) +} + +// In order for 'go test' to run this suite, we need to create +// a normal test function and pass our suite to suite.Run +func TestJaasSuite(t *testing.T) { + suite.Run(t, new(JaasSuite)) +} diff --git a/internal/juju/mock_test.go b/internal/juju/mock_test.go index 506c13ac..818b80e7 100644 --- a/internal/juju/mock_test.go +++ b/internal/juju/mock_test.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/terraform-provider-juju/internal/juju (interfaces: SharedClient,ClientAPIClient,ApplicationAPIClient,ModelConfigAPIClient,ResourceAPIClient,SecretAPIClient) +// Source: github.com/juju/terraform-provider-juju/internal/juju (interfaces: SharedClient,ClientAPIClient,ApplicationAPIClient,ModelConfigAPIClient,ResourceAPIClient,SecretAPIClient,JaasAPIClient) // // Generated by this command: // -// mockgen -package juju -destination mock_test.go github.com/juju/terraform-provider-juju/internal/juju SharedClient,ClientAPIClient,ApplicationAPIClient,ModelConfigAPIClient,ResourceAPIClient,SecretAPIClient +// mockgen -package juju -destination mock_test.go github.com/juju/terraform-provider-juju/internal/juju SharedClient,ClientAPIClient,ApplicationAPIClient,ModelConfigAPIClient,ResourceAPIClient,SecretAPIClient,JaasAPIClient // // Package juju is a generated GoMock package. @@ -12,6 +12,7 @@ package juju import ( reflect "reflect" + params "github.com/canonical/jimm-go-sdk/v3/api/params" charm "github.com/juju/charm/v12" api "github.com/juju/juju/api" application "github.com/juju/juju/api/client/application" @@ -23,7 +24,7 @@ import ( model "github.com/juju/juju/core/model" resources0 "github.com/juju/juju/core/resources" secrets0 "github.com/juju/juju/core/secrets" - params "github.com/juju/juju/rpc/params" + params0 "github.com/juju/juju/rpc/params" names "github.com/juju/names/v5" gomock "go.uber.org/mock/gomock" ) @@ -221,10 +222,10 @@ func (m *MockClientAPIClient) EXPECT() *MockClientAPIClientMockRecorder { } // Status mocks base method. -func (m *MockClientAPIClient) Status(arg0 *client.StatusArgs) (*params.FullStatus, error) { +func (m *MockClientAPIClient) Status(arg0 *client.StatusArgs) (*params0.FullStatus, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Status", arg0) - ret0, _ := ret[0].(*params.FullStatus) + ret0, _ := ret[0].(*params0.FullStatus) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -274,10 +275,10 @@ func (mr *MockApplicationAPIClientMockRecorder) AddUnits(arg0 any) *gomock.Call } // ApplicationsInfo mocks base method. -func (m *MockApplicationAPIClient) ApplicationsInfo(arg0 []names.ApplicationTag) ([]params.ApplicationInfoResult, error) { +func (m *MockApplicationAPIClient) ApplicationsInfo(arg0 []names.ApplicationTag) ([]params0.ApplicationInfoResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ApplicationsInfo", arg0) - ret0, _ := ret[0].([]params.ApplicationInfoResult) + ret0, _ := ret[0].([]params0.ApplicationInfoResult) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -303,10 +304,10 @@ func (mr *MockApplicationAPIClientMockRecorder) Deploy(arg0 any) *gomock.Call { } // DestroyApplications mocks base method. -func (m *MockApplicationAPIClient) DestroyApplications(arg0 application.DestroyApplicationsParams) ([]params.DestroyApplicationResult, error) { +func (m *MockApplicationAPIClient) DestroyApplications(arg0 application.DestroyApplicationsParams) ([]params0.DestroyApplicationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DestroyApplications", arg0) - ret0, _ := ret[0].([]params.DestroyApplicationResult) + ret0, _ := ret[0].([]params0.DestroyApplicationResult) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -318,10 +319,10 @@ func (mr *MockApplicationAPIClientMockRecorder) DestroyApplications(arg0 any) *g } // DestroyUnits mocks base method. -func (m *MockApplicationAPIClient) DestroyUnits(arg0 application.DestroyUnitsParams) ([]params.DestroyUnitResult, error) { +func (m *MockApplicationAPIClient) DestroyUnits(arg0 application.DestroyUnitsParams) ([]params0.DestroyUnitResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DestroyUnits", arg0) - ret0, _ := ret[0].([]params.DestroyUnitResult) + ret0, _ := ret[0].([]params0.DestroyUnitResult) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -333,7 +334,7 @@ func (mr *MockApplicationAPIClientMockRecorder) DestroyUnits(arg0 any) *gomock.C } // Expose mocks base method. -func (m *MockApplicationAPIClient) Expose(arg0 string, arg1 map[string]params.ExposedEndpoint) error { +func (m *MockApplicationAPIClient) Expose(arg0 string, arg1 map[string]params0.ExposedEndpoint) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Expose", arg0, arg1) ret0, _ := ret[0].(error) @@ -347,10 +348,10 @@ func (mr *MockApplicationAPIClientMockRecorder) Expose(arg0, arg1 any) *gomock.C } // Get mocks base method. -func (m *MockApplicationAPIClient) Get(arg0, arg1 string) (*params.ApplicationGetResults, error) { +func (m *MockApplicationAPIClient) Get(arg0, arg1 string) (*params0.ApplicationGetResults, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Get", arg0, arg1) - ret0, _ := ret[0].(*params.ApplicationGetResults) + ret0, _ := ret[0].(*params0.ApplicationGetResults) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -397,7 +398,7 @@ func (mr *MockApplicationAPIClientMockRecorder) GetConstraints(arg0 ...any) *gom } // MergeBindings mocks base method. -func (m *MockApplicationAPIClient) MergeBindings(arg0 params.ApplicationMergeBindingsArgs) error { +func (m *MockApplicationAPIClient) MergeBindings(arg0 params0.ApplicationMergeBindingsArgs) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "MergeBindings", arg0) ret0, _ := ret[0].(error) @@ -411,10 +412,10 @@ func (mr *MockApplicationAPIClientMockRecorder) MergeBindings(arg0 any) *gomock. } // ScaleApplication mocks base method. -func (m *MockApplicationAPIClient) ScaleApplication(arg0 application.ScaleApplicationParams) (params.ScaleApplicationResult, error) { +func (m *MockApplicationAPIClient) ScaleApplication(arg0 application.ScaleApplicationParams) (params0.ScaleApplicationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ScaleApplication", arg0) - ret0, _ := ret[0].(params.ScaleApplicationResult) + ret0, _ := ret[0].(params0.ScaleApplicationResult) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -682,3 +683,69 @@ func (mr *MockSecretAPIClientMockRecorder) UpdateSecret(arg0, arg1, arg2, arg3, mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateSecret", reflect.TypeOf((*MockSecretAPIClient)(nil).UpdateSecret), arg0, arg1, arg2, arg3, arg4, arg5) } + +// MockJaasAPIClient is a mock of JaasAPIClient interface. +type MockJaasAPIClient struct { + ctrl *gomock.Controller + recorder *MockJaasAPIClientMockRecorder +} + +// MockJaasAPIClientMockRecorder is the mock recorder for MockJaasAPIClient. +type MockJaasAPIClientMockRecorder struct { + mock *MockJaasAPIClient +} + +// NewMockJaasAPIClient creates a new mock instance. +func NewMockJaasAPIClient(ctrl *gomock.Controller) *MockJaasAPIClient { + mock := &MockJaasAPIClient{ctrl: ctrl} + mock.recorder = &MockJaasAPIClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockJaasAPIClient) EXPECT() *MockJaasAPIClientMockRecorder { + return m.recorder +} + +// AddRelation mocks base method. +func (m *MockJaasAPIClient) AddRelation(arg0 *params.AddRelationRequest) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddRelation", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddRelation indicates an expected call of AddRelation. +func (mr *MockJaasAPIClientMockRecorder) AddRelation(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddRelation", reflect.TypeOf((*MockJaasAPIClient)(nil).AddRelation), arg0) +} + +// ListRelationshipTuples mocks base method. +func (m *MockJaasAPIClient) ListRelationshipTuples(arg0 *params.ListRelationshipTuplesRequest) (*params.ListRelationshipTuplesResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListRelationshipTuples", arg0) + ret0, _ := ret[0].(*params.ListRelationshipTuplesResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListRelationshipTuples indicates an expected call of ListRelationshipTuples. +func (mr *MockJaasAPIClientMockRecorder) ListRelationshipTuples(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListRelationshipTuples", reflect.TypeOf((*MockJaasAPIClient)(nil).ListRelationshipTuples), arg0) +} + +// RemoveRelation mocks base method. +func (m *MockJaasAPIClient) RemoveRelation(arg0 *params.RemoveRelationRequest) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveRelation", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveRelation indicates an expected call of RemoveRelation. +func (mr *MockJaasAPIClientMockRecorder) RemoveRelation(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveRelation", reflect.TypeOf((*MockJaasAPIClient)(nil).RemoveRelation), arg0) +} diff --git a/internal/juju/offers.go b/internal/juju/offers.go index 864d9a5a..b8428417 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -10,7 +10,6 @@ import ( "strings" "time" - "github.com/juju/juju/api/client/application" apiapplication "github.com/juju/juju/api/client/application" "github.com/juju/juju/api/client/applicationoffers" apiclient "github.com/juju/juju/api/client/client" @@ -103,7 +102,7 @@ func (c offersClient) CreateOffer(input *CreateOfferInput) (*CreateOfferResponse return nil, append(errs, err) } defer func() { _ = modelConn.Close() }() - applicationClient := application.NewClient(modelConn) + applicationClient := apiapplication.NewClient(modelConn) // wait for the app to be available ctx, cancel := context.WithTimeout(context.Background(), OfferAppAvailableTimeout) diff --git a/internal/juju/package_test.go b/internal/juju/package_test.go index 8a3f724f..4754e0d1 100644 --- a/internal/juju/package_test.go +++ b/internal/juju/package_test.go @@ -3,5 +3,5 @@ package juju_test -//go:generate go run go.uber.org/mock/mockgen -package juju -destination mock_test.go github.com/juju/terraform-provider-juju/internal/juju SharedClient,ClientAPIClient,ApplicationAPIClient,ModelConfigAPIClient,ResourceAPIClient,SecretAPIClient +//go:generate go run go.uber.org/mock/mockgen -package juju -destination mock_test.go github.com/juju/terraform-provider-juju/internal/juju SharedClient,ClientAPIClient,ApplicationAPIClient,ModelConfigAPIClient,ResourceAPIClient,SecretAPIClient,JaasAPIClient //go:generate go run go.uber.org/mock/mockgen -package juju -destination jujuapi_mock_test.go github.com/juju/juju/api Connection diff --git a/internal/juju/secrets_test.go b/internal/juju/secrets_test.go index f5bfd29f..e74be5b4 100644 --- a/internal/juju/secrets_test.go +++ b/internal/juju/secrets_test.go @@ -29,7 +29,7 @@ func (s *SecretSuite) SetupTest() {} func (s *SecretSuite) setupMocks(t *testing.T) *gomock.Controller { s.testModelName = "test-secret-model" - ctlr := s.JujuSuite.setupMocks(t) + ctlr := s.JujuSuite.setupMocks(t, &s.testModelName) s.mockSecretClient = NewMockSecretAPIClient(ctlr) return ctlr From 8a5c00043b62857e65d17d1f752af06da446a800 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Tue, 6 Aug 2024 15:19:53 +0200 Subject: [PATCH 3/9] feat: add missing godocs --- internal/juju/interfaces.go | 1 + internal/juju/jaas.go | 5 ++++- internal/juju/jaas_test.go | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/juju/interfaces.go b/internal/juju/interfaces.go index be8460a4..f02f67bd 100644 --- a/internal/juju/interfaces.go +++ b/internal/juju/interfaces.go @@ -78,6 +78,7 @@ type SecretAPIClient interface { RevokeSecret(uri *secrets.URI, name string, apps []string) ([]error, error) } +// JaasAPIClient defines the set of methods that the JAAS API provides. type JaasAPIClient interface { ListRelationshipTuples(req *jaasparams.ListRelationshipTuplesRequest) (*jaasparams.ListRelationshipTuplesResponse, error) AddRelation(req *jaasparams.AddRelationRequest) error diff --git a/internal/juju/jaas.go b/internal/juju/jaas.go index 5d1ef495..457ab759 100644 --- a/internal/juju/jaas.go +++ b/internal/juju/jaas.go @@ -1,4 +1,4 @@ -// Copyright 2023 Canonical Ltd. +// Copyright 2024 Canonical Ltd. // Licensed under the Apache License, Version 2.0, see LICENCE file for details. package juju @@ -25,6 +25,7 @@ func newJaasClient(sc SharedClient) *jaasClient { } } +// AddRelations attempts to create the provided slice of relationship tuples. func (jc *jaasClient) AddRelations(tuples []params.RelationshipTuple) error { conn, err := jc.GetConnection(nil) if err != nil { @@ -38,6 +39,7 @@ func (jc *jaasClient) AddRelations(tuples []params.RelationshipTuple) error { return cl.AddRelation(&req) } +// DeleteRelations attempts to delete the provided slice of relationship tuples. func (jc *jaasClient) DeleteRelations(tuples []params.RelationshipTuple) error { conn, err := jc.GetConnection(nil) if err != nil { @@ -51,6 +53,7 @@ func (jc *jaasClient) DeleteRelations(tuples []params.RelationshipTuple) error { 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) { if tuple == nil { return nil, errors.New("add relation request nil") diff --git a/internal/juju/jaas_test.go b/internal/juju/jaas_test.go index f0589a35..9a17915a 100644 --- a/internal/juju/jaas_test.go +++ b/internal/juju/jaas_test.go @@ -1,4 +1,4 @@ -// Copyright 2023 Canonical Ltd. +// Copyright 2024 Canonical Ltd. // Licensed under the Apache License, Version 2.0, see LICENCE file for details. package juju From da3cffdd925df975494f6f675ff483e12aae4a86 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Wed, 7 Aug 2024 10:04:23 +0200 Subject: [PATCH 4/9] feat: address PR changes - Don't leak api types outside of the jaas client. - Cleanup tests --- internal/juju/common_test.go | 4 +--- internal/juju/jaas.go | 44 ++++++++++++++++++++++++++++------- internal/juju/jaas_test.go | 38 ++++++++++++++---------------- internal/juju/secrets_test.go | 32 ++++++++++++------------- 4 files changed, 70 insertions(+), 48 deletions(-) diff --git a/internal/juju/common_test.go b/internal/juju/common_test.go index c392ddb8..78a4e766 100644 --- a/internal/juju/common_test.go +++ b/internal/juju/common_test.go @@ -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) diff --git a/internal/juju/jaas.go b/internal/juju/jaas.go index 457ab759..0170f89f 100644 --- a/internal/juju/jaas.go +++ b/internal/juju/jaas.go @@ -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 { @@ -25,8 +24,35 @@ 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 @@ -34,13 +60,14 @@ func (jc *jaasClient) AddRelations(tuples []params.RelationshipTuple) error { 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 { +// The caller is expected to populate the slice so that `len(tuples) > 0`. +func (jc *jaasClient) DeleteRelations(tuples []JaasTuple) error { conn, err := jc.GetConnection(nil) if err != nil { return err @@ -48,13 +75,14 @@ func (jc *jaasClient) DeleteRelations(tuples []params.RelationshipTuple) error { 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) { +// The caller is expected to provide a non-nil tuple. +func (jc *jaasClient) ReadRelations(tuple *JaasTuple) ([]params.RelationshipTuple, error) { if tuple == nil { return nil, errors.New("add relation request nil") } @@ -67,7 +95,7 @@ func (jc *jaasClient) ReadRelations(tuple *params.RelationshipTuple) ([]params.R client := jc.getJaasApiClient(conn) relations := make([]params.RelationshipTuple, 0) - req := ¶ms.ListRelationshipTuplesRequest{Tuple: *tuple} + req := ¶ms.ListRelationshipTuplesRequest{Tuple: toAPITuple(*tuple)} for { resp, err := client.ListRelationshipTuples(req) if err != nil { diff --git a/internal/juju/jaas_test.go b/internal/juju/jaas_test.go index 9a17915a..c9da2572 100644 --- a/internal/juju/jaas_test.go +++ b/internal/juju/jaas_test.go @@ -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 @@ -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) @@ -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) @@ -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 := ¶ms.ListRelationshipTuplesRequest{Tuple: tuple} + req := ¶ms.ListRelationshipTuplesRequest{Tuple: toAPITuple(tuple)} respWithToken := ¶ms.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 := ¶ms.ListRelationshipTuplesRequest{Tuple: tuple, ContinuationToken: "token"} + reqWithToken := ¶ms.ListRelationshipTuplesRequest{Tuple: toAPITuple(tuple), ContinuationToken: "token"} respWithoutToken := ¶ms.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) diff --git a/internal/juju/secrets_test.go b/internal/juju/secrets_test.go index e74be5b4..2a8a3782 100644 --- a/internal/juju/secrets_test.go +++ b/internal/juju/secrets_test.go @@ -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 @@ -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", @@ -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", @@ -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, }) @@ -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) @@ -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, @@ -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) @@ -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, @@ -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) @@ -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) } @@ -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) From 21e66fda3c044e54ea22838355fc25ac69821151 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Mon, 12 Aug 2024 09:12:04 +0200 Subject: [PATCH 5/9] feat: add checks for empty slice --- internal/juju/jaas.go | 11 +++++++++-- internal/juju/jaas_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/internal/juju/jaas.go b/internal/juju/jaas.go index 0170f89f..e59d13f6 100644 --- a/internal/juju/jaas.go +++ b/internal/juju/jaas.go @@ -4,9 +4,10 @@ package juju import ( + "errors" + "github.com/canonical/jimm-go-sdk/v3/api" "github.com/canonical/jimm-go-sdk/v3/api/params" - "github.com/juju/errors" jujuapi "github.com/juju/juju/api" ) @@ -53,6 +54,9 @@ func toAPITuple(tuple JaasTuple) params.RelationshipTuple { // AddRelations attempts to create the provided slice of relationship tuples. // The caller is expected to populate the slice so that `len(tuples) > 0`. func (jc *jaasClient) AddRelations(tuples []JaasTuple) error { + if len(tuples) == 0 { + return errors.New("empty slice of tuples") + } conn, err := jc.GetConnection(nil) if err != nil { return err @@ -68,6 +72,9 @@ func (jc *jaasClient) AddRelations(tuples []JaasTuple) error { // DeleteRelations attempts to delete the provided slice of relationship tuples. // The caller is expected to populate the slice so that `len(tuples) > 0`. func (jc *jaasClient) DeleteRelations(tuples []JaasTuple) error { + if len(tuples) == 0 { + return errors.New("empty slice of tuples") + } conn, err := jc.GetConnection(nil) if err != nil { return err @@ -84,7 +91,7 @@ func (jc *jaasClient) DeleteRelations(tuples []JaasTuple) error { // The caller is expected to provide a non-nil tuple. func (jc *jaasClient) ReadRelations(tuple *JaasTuple) ([]params.RelationshipTuple, error) { if tuple == nil { - return nil, errors.New("add relation request nil") + return nil, errors.New("read relation tuple is nil") } conn, err := jc.GetConnection(nil) diff --git a/internal/juju/jaas_test.go b/internal/juju/jaas_test.go index c9da2572..40944cb4 100644 --- a/internal/juju/jaas_test.go +++ b/internal/juju/jaas_test.go @@ -4,6 +4,7 @@ package juju import ( + "errors" "testing" "github.com/canonical/jimm-go-sdk/v3/api/params" @@ -56,6 +57,14 @@ func (s *JaasSuite) TestAddRelations() { s.Require().NoError(err) } +func (s *JaasSuite) TestAddRelationsEmptySlice() { + expectedErr := errors.New("empty slice of tuples") + client := s.getJaasClient() + err := client.AddRelations([]JaasTuple{}) + s.Require().Error(err) + s.Assert().Equal(expectedErr, err) +} + func (s *JaasSuite) TestDeleteRelations() { ctlr := s.setupMocks(s.T()) defer ctlr.Finish() @@ -77,6 +86,14 @@ func (s *JaasSuite) TestDeleteRelations() { s.Require().NoError(err) } +func (s *JaasSuite) TestDeleteRelationsEmptySlice() { + expectedErr := errors.New("empty slice of tuples") + client := s.getJaasClient() + err := client.DeleteRelations([]JaasTuple{}) + s.Require().Error(err) + s.Assert().Equal(expectedErr, err) +} + func (s *JaasSuite) TestReadRelations() { ctlr := s.setupMocks(s.T()) defer ctlr.Finish() @@ -107,6 +124,14 @@ func (s *JaasSuite) TestReadRelations() { s.Require().Len(relations, 2) } +func (s *JaasSuite) TestReadRelationsEmptyTuple() { + expectedErr := errors.New("read relation tuple is nil") + client := s.getJaasClient() + _, err := client.ReadRelations(nil) + s.Require().Error(err) + s.Assert().Equal(expectedErr, err) +} + // In order for 'go test' to run this suite, we need to create // a normal test function and pass our suite to suite.Run func TestJaasSuite(t *testing.T) { From c711dad715ee532c7174b51e34d61682cc848e69 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Mon, 12 Aug 2024 11:35:35 +0200 Subject: [PATCH 6/9] feat: revert go version to 1.21 --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 2796d3fe..3829e9fa 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/juju/terraform-provider-juju -go 1.22.3 +go 1.21 require ( github.com/bflad/tfproviderlint v0.30.0 From 7a879a0a16f841a74a0e6221ed39f6123785188d Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Tue, 13 Aug 2024 10:47:05 +0200 Subject: [PATCH 7/9] feat: avoid infinite loop by using ctx Updated godocs --- internal/juju/jaas.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/juju/jaas.go b/internal/juju/jaas.go index e59d13f6..de8d5e7a 100644 --- a/internal/juju/jaas.go +++ b/internal/juju/jaas.go @@ -4,6 +4,7 @@ package juju import ( + "context" "errors" "github.com/canonical/jimm-go-sdk/v3/api" @@ -52,7 +53,7 @@ func toAPITuple(tuple JaasTuple) params.RelationshipTuple { } // AddRelations attempts to create the provided slice of relationship tuples. -// The caller is expected to populate the slice so that `len(tuples) > 0`. +// An empty slice of tuples will return an error. func (jc *jaasClient) AddRelations(tuples []JaasTuple) error { if len(tuples) == 0 { return errors.New("empty slice of tuples") @@ -70,7 +71,7 @@ func (jc *jaasClient) AddRelations(tuples []JaasTuple) error { } // DeleteRelations attempts to delete the provided slice of relationship tuples. -// The caller is expected to populate the slice so that `len(tuples) > 0`. +// An empty slice of tuples will return an error. func (jc *jaasClient) DeleteRelations(tuples []JaasTuple) error { if len(tuples) == 0 { return errors.New("empty slice of tuples") @@ -88,8 +89,8 @@ func (jc *jaasClient) DeleteRelations(tuples []JaasTuple) error { } // ReadRelations attempts to read relations that match the criteria defined by `tuple`. -// The caller is expected to provide a non-nil tuple. -func (jc *jaasClient) ReadRelations(tuple *JaasTuple) ([]params.RelationshipTuple, error) { +// An nil tuple pointer is invalid and will return an error. +func (jc *jaasClient) ReadRelations(ctx context.Context, tuple *JaasTuple) ([]params.RelationshipTuple, error) { if tuple == nil { return nil, errors.New("read relation tuple is nil") } @@ -118,5 +119,10 @@ func (jc *jaasClient) ReadRelations(tuple *JaasTuple) ([]params.RelationshipTupl return relations, nil } req.ContinuationToken = resp.ContinuationToken + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } } } From f1aead1bd577bb0af2ae595d04d0aa3e94a1c1ed Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Thu, 15 Aug 2024 09:55:31 +0200 Subject: [PATCH 8/9] feat: added test for cancelled context --- internal/juju/client.go | 1 - internal/juju/jaas_test.go | 30 ++++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/internal/juju/client.go b/internal/juju/client.go index 97d7fe1b..c8ba7dbe 100644 --- a/internal/juju/client.go +++ b/internal/juju/client.go @@ -58,7 +58,6 @@ type Client struct { // JAAS controllers offer additional functionality for permission management. func (c Client) IsJAAS() bool { return c.isJAAS() - } type jujuModel struct { diff --git a/internal/juju/jaas_test.go b/internal/juju/jaas_test.go index 40944cb4..50439225 100644 --- a/internal/juju/jaas_test.go +++ b/internal/juju/jaas_test.go @@ -4,6 +4,7 @@ package juju import ( + "context" "errors" "testing" @@ -119,7 +120,7 @@ func (s *JaasSuite) TestReadRelations() { ).Return(respWithoutToken, nil) client := s.getJaasClient() - relations, err := client.ReadRelations(&tuple) + relations, err := client.ReadRelations(context.Background(), &tuple) s.Require().NoError(err) s.Require().Len(relations, 2) } @@ -127,7 +128,32 @@ func (s *JaasSuite) TestReadRelations() { func (s *JaasSuite) TestReadRelationsEmptyTuple() { expectedErr := errors.New("read relation tuple is nil") client := s.getJaasClient() - _, err := client.ReadRelations(nil) + _, err := client.ReadRelations(context.Background(), nil) + s.Require().Error(err) + s.Assert().Equal(expectedErr, err) +} + +func (s *JaasSuite) TestReadRelationsCancelledContext() { + ctlr := s.setupMocks(s.T()) + defer ctlr.Finish() + + tuple := JaasTuple{Object: "object-1", Relation: "relation", Target: "target-1"} + req := ¶ms.ListRelationshipTuplesRequest{Tuple: toAPITuple(tuple)} + respWithToken := ¶ms.ListRelationshipTuplesResponse{ + Tuples: []params.RelationshipTuple{toAPITuple(tuple)}, + ContinuationToken: "token", + } + s.mockJaasClient.EXPECT().ListRelationshipTuples( + req, + ).Return(respWithToken, nil) + + expectedErr := errors.New("context canceled") + ctx := context.Background() + ctx, cancelFunc := context.WithCancel(ctx) + cancelFunc() + + client := s.getJaasClient() + _, err := client.ReadRelations(ctx, &tuple) s.Require().Error(err) s.Assert().Equal(expectedErr, err) } From 960de199e5eef684b26c9b468c90b44b0fb20580 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Fri, 16 Aug 2024 08:53:43 +0200 Subject: [PATCH 9/9] feat: simplify test code --- internal/juju/jaas_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/juju/jaas_test.go b/internal/juju/jaas_test.go index 50439225..a46b026b 100644 --- a/internal/juju/jaas_test.go +++ b/internal/juju/jaas_test.go @@ -38,8 +38,7 @@ func (s *JaasSuite) getJaasClient() jaasClient { } func (s *JaasSuite) TestAddRelations() { - ctlr := s.setupMocks(s.T()) - defer ctlr.Finish() + defer s.setupMocks(s.T()).Finish() tuples := []JaasTuple{ {Object: "object-1", Relation: "relation", Target: "target-1"}, @@ -67,8 +66,7 @@ func (s *JaasSuite) TestAddRelationsEmptySlice() { } func (s *JaasSuite) TestDeleteRelations() { - ctlr := s.setupMocks(s.T()) - defer ctlr.Finish() + defer s.setupMocks(s.T()).Finish() tuples := []JaasTuple{ {Object: "object-1", Relation: "relation", Target: "target-1"}, @@ -96,8 +94,7 @@ func (s *JaasSuite) TestDeleteRelationsEmptySlice() { } func (s *JaasSuite) TestReadRelations() { - ctlr := s.setupMocks(s.T()) - defer ctlr.Finish() + defer s.setupMocks(s.T()).Finish() 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. @@ -134,8 +131,7 @@ func (s *JaasSuite) TestReadRelationsEmptyTuple() { } func (s *JaasSuite) TestReadRelationsCancelledContext() { - ctlr := s.setupMocks(s.T()) - defer ctlr.Finish() + defer s.setupMocks(s.T()).Finish() tuple := JaasTuple{Object: "object-1", Relation: "relation", Target: "target-1"} req := ¶ms.ListRelationshipTuplesRequest{Tuple: toAPITuple(tuple)}