Skip to content

Commit

Permalink
refactor(addhostedcloud): refactors AddHostedCloud
Browse files Browse the repository at this point in the history 
AddHostedCloud expected the host cloud/region to really be host cloud-type/region, which is not what
the terraform provider specifies. This PR changes it so that one can specify cloud-name/region-name
as well as cloud-type/region-name when adding a host cloud.
  • Loading branch information
@alesstimec
alesstimec committed Dec 19, 2024
1 parent 5c5ee6c commit 3db38f3
Show file tree
Hide file tree
Showing 6 changed files with 247 additions and 34 deletions.
2 changes: 1 addition & 1 deletion cmd/jimmctl/cmd/addcloudtocontroller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (s *addCloudToControllerSuite) SetUpTest(c *gc.C) {
Regions: []dbmodel.CloudRegion{{Name: "default", CloudName: "test-cloud"}},
})
c.Assert(err, gc.IsNil)
region, err := s.JIMM.Database.FindRegion(context.Background(), "kubernetes", "default")
region, err := s.JIMM.Database.FindRegionByCloudType(context.Background(), "kubernetes", "default")
c.Assert(err, gc.IsNil)

// We grant user bob administrator access to JIMM and the added
Expand Down
14 changes: 0 additions & 14 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,10 +590,6 @@ github.com/juju/clock v1.1.1 h1:NvgHG9DQmOpBevgt6gzkyimdWBooLXDy1cQn89qJzBI=
github.com/juju/clock v1.1.1/go.mod h1:HIBvJ8kiV/n7UHwKuCkdYL4l/MDECztHR2sAvWDxxf0=
github.com/juju/cmd v0.0.0-20171107070456-e74f39857ca0/go.mod h1:yWJQHl73rdSX4DHVKGqkAip+huBslxRwS8m9CrOLq18=
github.com/juju/cmd/v3 v3.0.0-20220202061353-b1cc80b193b0/go.mod h1:EoGJiEG+vbMwO9l+Es0SDTlaQPjH6nLcnnc4NfZB3cY=
github.com/juju/cmd/v3 v3.0.16 h1:P/tG4BPtE+MsmECtnFx+KBC8NFi1ESvHbZ8NzMAll1g=
github.com/juju/cmd/v3 v3.0.16/go.mod h1:lGtDvm2BG+FKnIS8yY/vrhxQNX9imnL6bPIYGSTchuI=
github.com/juju/cmd/v3 v3.1.0 h1:u8aXkeZI/BWSK310oOVMgFq66odDY7QGbL+hU1wp1OA=
github.com/juju/cmd/v3 v3.1.0/go.mod h1:lGtDvm2BG+FKnIS8yY/vrhxQNX9imnL6bPIYGSTchuI=
github.com/juju/cmd/v3 v3.1.1 h1:mhXjnm/tj1uYieTd6zc+lAyCrXsETrD+WdsTI6gAM3c=
github.com/juju/cmd/v3 v3.1.1/go.mod h1:lGtDvm2BG+FKnIS8yY/vrhxQNX9imnL6bPIYGSTchuI=
github.com/juju/collections v0.0.0-20200605021417-0d0ec82b7271/go.mod h1:5XgO71dV1JClcOJE+4dzdn4HrI5LiyKd7PlVG6eZYhY=
Expand Down Expand Up @@ -1145,8 +1141,6 @@ golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf
golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4=
golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
golang.org/x/crypto v0.20.0/go.mod h1:Xwo95rrVNIoSMx9wa1JroENMToLWn3RNVrTBpLHgZPQ=
golang.org/x/crypto v0.29.0 h1:L5SG1JTTXupVV3n6sUqMTeWbjAyfPwoda2DLX8J8FrQ=
golang.org/x/crypto v0.29.0/go.mod h1:+F4F4N5hv6v38hfeYwTdx20oUvLLc+QfrE9Ax9HtgRg=
golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
Expand Down Expand Up @@ -1267,8 +1261,6 @@ golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.9.0 h1:fEo0HyrW1GIgZdpbhCRO0PkJajUS5H9IFUztCgEo2jQ=
golang.org/x/sync v0.9.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
Expand Down Expand Up @@ -1341,8 +1333,6 @@ golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s=
golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
Expand All @@ -1356,8 +1346,6 @@ golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U=
golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0=
golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk=
golang.org/x/term v0.26.0 h1:WEQa6V3Gja/BhNxg540hBip/kkaYtRg3cxg4oXSw4AU=
golang.org/x/term v0.26.0/go.mod h1:Si5m1o57C5nBNQo5z1iq+XDijt21BDBDp2bK0QI8e3E=
golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q=
golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand All @@ -1375,8 +1363,6 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
golang.org/x/text v0.20.0 h1:gK/Kv2otX8gz+wn7Rmb3vT96ZwuoxnQlY+HlJVj7Qug=
golang.org/x/text v0.20.0/go.mod h1:D4IsuqiFMhST5bX19pQ9ikHC2GsaKyk/oF+pn3ducp4=
golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
Expand Down
31 changes: 27 additions & 4 deletions internal/db/cloud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@ func (d *Database) AddCloudRegion(ctx context.Context, cr *dbmodel.CloudRegion)
return nil
}

// FindRegion finds a region with the given name on a cloud with the given
// provider type.
func (d *Database) FindRegion(ctx context.Context, providerType, name string) (_ *dbmodel.CloudRegion, err error) {
// FindRegionByCloudType finds a region with the given name on a cloud with the given
// cloud type.
func (d *Database) FindRegionByCloudType(ctx context.Context, providerType, regionName string) (_ *dbmodel.CloudRegion, err error) {
const op = errors.Op("db.FindRegion")
if err := d.ready(); err != nil {
return nil, errors.E(op, err)
Expand All @@ -166,7 +166,30 @@ func (d *Database) FindRegion(ctx context.Context, providerType, name string) (_

db := d.DB.WithContext(ctx)
db = db.Preload("Cloud").Preload("Controllers").Preload("Controllers.Controller")
db = db.Model(dbmodel.CloudRegion{}).Joins("INNER JOIN clouds ON clouds.name = cloud_regions.cloud_name").Where("clouds.type = ? AND cloud_regions.name = ?", providerType, name)
db = db.Model(dbmodel.CloudRegion{}).Joins("INNER JOIN clouds ON clouds.name = cloud_regions.cloud_name").Where("clouds.type = ? AND cloud_regions.name = ?", providerType, regionName)

var region dbmodel.CloudRegion
if err := db.First(&region).Error; err != nil {
return nil, errors.E(op, dbError(err))
}
return &region, nil
}

// FindRegionByCloudName finds a region with the given name on a cloud with the given
// name.
func (d *Database) FindRegionByCloudName(ctx context.Context, cloudName, regionName string) (_ *dbmodel.CloudRegion, err error) {
const op = errors.Op("db.FindRegion")
if err := d.ready(); err != nil {
return nil, errors.E(op, err)
}

durationObserver := servermon.DurationObserver(servermon.DBQueryDurationHistogram, string(op))
defer durationObserver()
defer servermon.ErrorCounter(servermon.DBQueryErrorCount, &err, string(op))

db := d.DB.WithContext(ctx)
db = db.Preload("Cloud").Preload("Controllers").Preload("Controllers.Controller")
db = db.Model(dbmodel.CloudRegion{}).Joins("INNER JOIN clouds ON clouds.name = cloud_regions.cloud_name").Where("clouds.name = ? AND cloud_regions.name = ?", cloudName, regionName)

var region dbmodel.CloudRegion
if err := db.First(&region).Error; err != nil {
Expand Down
53 changes: 49 additions & 4 deletions internal/db/cloud_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,13 @@ func TestFindRegionUnconfiguredDatabase(t *testing.T) {
c := qt.New(t)

var d db.Database
cr, err := d.FindRegion(context.Background(), "", "")
cr, err := d.FindRegionByCloudType(context.Background(), "", "")
c.Check(err, qt.ErrorMatches, `database not configured`)
c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeServerConfiguration)
c.Check(cr, qt.IsNil)
}

func (s *dbSuite) TestFindRegion(c *qt.C) {
func (s *dbSuite) TestFindRegionByCloudType(c *qt.C) {
ctx := context.Background()

err := s.Database.Migrate(ctx, false)
Expand All @@ -246,7 +246,7 @@ controllers:
`)
env.PopulateDB(c, s.Database)

cr, err := s.Database.FindRegion(ctx, "testp", "test-region")
cr, err := s.Database.FindRegionByCloudType(ctx, "testp", "test-region")
c.Assert(err, qt.IsNil)
c.Check(cr, jimmtest.DBObjectEquals, &dbmodel.CloudRegion{
Cloud: dbmodel.Cloud{
Expand All @@ -265,7 +265,52 @@ controllers:
}},
})

_, err = s.Database.FindRegion(ctx, "no-such", "region")
_, err = s.Database.FindRegionByCloudType(ctx, "no-such", "region")
c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeNotFound)
}
func (s *dbSuite) TestFindRegionByCloudName(c *qt.C) {
ctx := context.Background()

err := s.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)

env := jimmtest.ParseEnvironment(c, `clouds:
- name: test
type: testp
regions:
- name: test-region
controllers:
- name: test
uuid: 00000001-0000-0000-0000-000000000001
cloud: test
region: test-region
cloud-regions:
- cloud: test
region: test-region
priority: 1
`)
env.PopulateDB(c, s.Database)

cr, err := s.Database.FindRegionByCloudName(ctx, "test", "test-region")
c.Assert(err, qt.IsNil)
c.Check(cr, jimmtest.DBObjectEquals, &dbmodel.CloudRegion{
Cloud: dbmodel.Cloud{
Name: "test",
Type: "testp",
},
Name: "test-region",
Controllers: []dbmodel.CloudRegionControllerPriority{{
Controller: dbmodel.Controller{
Name: "test",
UUID: "00000001-0000-0000-0000-000000000001",
CloudName: "test",
CloudRegion: "test-region",
},
Priority: 1,
}},
})

_, err = s.Database.FindRegionByCloudName(ctx, "no-such", "region")
c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeNotFound)
}

Expand Down
59 changes: 50 additions & 9 deletions internal/jimm/cloud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,52 @@ func (j *JIMM) AddCloudToController(ctx context.Context, user *openfga.User, con
return nil
}

func (j *JIMM) determineHostCloudRegion(ctx context.Context, hostCloudRegion string) (*dbmodel.CloudRegion, error) {
// if the hostCloudRegion does not contain '/', then we consider it to be
// the cloud name
if !strings.Contains(hostCloudRegion, "/") {
cl := dbmodel.Cloud{
Name: hostCloudRegion,
}
if err := j.Database.GetCloud(ctx, &cl); err != nil {
return nil, errors.E(errors.CodeNotFound, "unable to find host cloud %q", hostCloudRegion)
}
if len(cl.Regions) > 1 {
return nil, errors.E(errors.CodeBadRequest, "unable to determine a unique region for host cloud %q - consider specifying the host cloud region", hostCloudRegion)
}
if len(cl.Regions) == 0 {
return nil, errors.E(errors.CodeBadRequest, "the host cloud %q does not have a valid region", hostCloudRegion)
}
return &cl.Regions[0], nil
}

parts := strings.Split(hostCloudRegion, "/")
if len(parts) != 2 || parts[0] == "" {
return nil, errors.E(errors.CodeBadRequest, fmt.Sprintf("invalid cloud/region format %q", hostCloudRegion))
}

findRegionFunctions := []func(context.Context, string, string) (*dbmodel.CloudRegion, error){
j.Database.FindRegionByCloudType,
j.Database.FindRegionByCloudName,
}

var err error
var region *dbmodel.CloudRegion
for _, findRegionFunction := range findRegionFunctions {
region, err = findRegionFunction(ctx, parts[0], parts[1])
if err == nil {
break
}
}
if err != nil {
if errors.ErrorCode(err) == errors.CodeNotFound {
return nil, errors.E(err, errors.CodeNotFound, fmt.Sprintf("unable to find cloud/region %q", hostCloudRegion))
}
return nil, err
}
return region, nil
}

// AddHostedCloud adds the cloud defined by the given tag and cloud to the
// JAAS system. The cloud will be created on a controller running on the
// requested host cloud-region and the cloud created there. If the given
Expand Down Expand Up @@ -213,14 +259,9 @@ func (j *JIMM) AddHostedCloud(ctx context.Context, user *openfga.User, tag names
if cloud.HostCloudRegion == "" {
return errors.E(op, errors.CodeCloudRegionRequired, "cloud host region not specified")
}
parts := strings.SplitN(cloud.HostCloudRegion, "/", 2)
if len(parts) != 2 || parts[0] == "" {
return errors.E(op, errors.CodeBadRequest, fmt.Sprintf("invalid cloud/region format %q", cloud.HostCloudRegion))
}
region, err := j.Database.FindRegion(ctx, parts[0], parts[1])
if errors.ErrorCode(err) == errors.CodeNotFound {
return errors.E(op, err, errors.CodeNotFound, fmt.Sprintf("unable to find cloud/region %q", cloud.HostCloudRegion))
} else if err != nil {

region, err := j.determineHostCloudRegion(ctx, cloud.HostCloudRegion)
if err != nil {
return errors.E(op, err)
}

Expand Down Expand Up @@ -728,7 +769,7 @@ func validateCloudRegion(ctx context.Context, db *db.Database, user *openfga.Use
return errors.E(errors.CodeIncompatibleClouds, fmt.Sprintf("cloud host region %q has invalid cloud/region format", cloud.HostCloudRegion))
}

region, err := db.FindRegion(ctx, parts[0], parts[1])
region, err := db.FindRegionByCloudType(ctx, parts[0], parts[1])
if err != nil {
if errors.ErrorCode(err) == errors.CodeNotFound {
return errors.E(errors.CodeIncompatibleClouds, fmt.Sprintf("unable to find cloud/region %q", cloud.HostCloudRegion))
Expand Down
Loading

0 comments on commit 3db38f3

Please sign in to comment.