From 9fd582db0cc7fdb3e96c1381b29b5e733463d037 Mon Sep 17 00:00:00 2001 From: Ales Stimec Date: Wed, 8 Jan 2025 15:48:41 +0100 Subject: [PATCH] fix(internal/jimm/controller.go): add default cloud region if none exist When adding a controller JIMM will query the controller for all known clouds and then add those clouds to its internal DB. If a cloud (e.g. MAAS or k8s) has no regions, JIMM would be unable to add a model to that cloud as it does not store association between clouds and controllers. To solve the issue we add a `default` region to clouds that do not have any regions and that cloud region will only have one controller associated with it. --- internal/jimm/applicationoffer_test.go | 2 +- internal/jimm/audit_log_test.go | 2 +- internal/jimm/controller.go | 18 +++++- internal/jimm/controller_test.go | 90 +++++++++++++++++++++++++- internal/jimm/export_test.go | 2 +- internal/jimm/group/group_test.go | 2 +- internal/jimm/jimm_test.go | 2 +- internal/jimm/model_cleanup_test.go | 2 +- internal/jimm/model_test.go | 2 +- internal/jimm/role/role_test.go | 2 +- internal/jimm/watcher_test.go | 2 +- 11 files changed, 115 insertions(+), 11 deletions(-) diff --git a/internal/jimm/applicationoffer_test.go b/internal/jimm/applicationoffer_test.go index 0f3f32e10..efe9dade6 100644 --- a/internal/jimm/applicationoffer_test.go +++ b/internal/jimm/applicationoffer_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm_test diff --git a/internal/jimm/audit_log_test.go b/internal/jimm/audit_log_test.go index 7983cb6f3..fcb44ae9d 100644 --- a/internal/jimm/audit_log_test.go +++ b/internal/jimm/audit_log_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm_test diff --git a/internal/jimm/controller.go b/internal/jimm/controller.go index feb54eeb3..a94144df9 100644 --- a/internal/jimm/controller.go +++ b/internal/jimm/controller.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm @@ -252,6 +252,22 @@ func (j *JIMM) AddController(ctx context.Context, user *openfga.User, ctl *dbmod dbClouds := convertJujuCloudsToDbClouds(clouds) + for i, cloud := range dbClouds { + // in case this cloud has no regions (as happens for + // some MAAS and k8s clouds), we create one region + // named `default` + if len(cloud.Regions) == 0 { + dbClouds[i].Regions = []dbmodel.CloudRegion{{ + CloudName: cloud.Name, + Name: "default", + }} + if cloud.Name == ctl.CloudName { + ctl.CloudRegion = "default" + } + + } + } + // TODO(ale8k): This shouldn't be necessary to check, but tests need updating // to set insecure credential store explicitly. if j.CredentialStore != nil { diff --git a/internal/jimm/controller_test.go b/internal/jimm/controller_test.go index 474c92176..fb66b497a 100644 --- a/internal/jimm/controller_test.go +++ b/internal/jimm/controller_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm_test @@ -176,6 +176,94 @@ func TestAddController(t *testing.T) { c.Check(ctl4, qt.CmpEquals(cmpopts.EquateEmpty(), cmpopts.IgnoreTypes(dbmodel.CloudRegion{})), ctl3) } +func TestAddControllerWithCloudWithoutRegions(t *testing.T) { + c := qt.New(t) + + api := &jimmtest.API{ + Clouds_: func(context.Context) (map[names.CloudTag]jujuparams.Cloud, error) { + clouds := map[names.CloudTag]jujuparams.Cloud{ + names.NewCloudTag("k8s"): { + Type: "kubernetes", + AuthTypes: []string{"userpass"}, + Endpoint: "https://k8s.example.com", + }, + } + return clouds, nil + }, + CloudInfo_: func(_ context.Context, tag names.CloudTag, ci *jujuparams.CloudInfo) error { + if tag.Id() != "k8s" { + c.Errorf("CloudInfo called for unexpected cloud %q", tag) + return errors.E("unexpected cloud") + } + ci.Type = "kubernetes" + ci.AuthTypes = []string{"userpass"} + ci.Endpoint = "https://k8s.example.com" + return nil + }, + ControllerModelSummary_: func(_ context.Context, ms *jujuparams.ModelSummary) error { + ms.Name = "controller" + ms.UUID = "5fddf0ed-83d5-47e8-ae7b-a4b27fc04a9f" + ms.Type = "iaas" + ms.ControllerUUID = jimmtest.DefaultControllerUUID + ms.IsController = true + ms.ProviderType = "ec2" + ms.DefaultSeries = "warty" + ms.CloudTag = "cloud-k8s" + ms.OwnerTag = "user-admin" + ms.Life = life.Value(state.Alive.String()) + ms.Status = jujuparams.EntityStatus{ + Status: "available", + } + ms.UserAccess = "admin" + ms.AgentVersion = newVersion("1.2.3") + return nil + }, + } + + j := jimmtest.NewJIMM(c, &jimm.Parameters{ + Dialer: &jimmtest.Dialer{ + API: api, + }, + }) + + ctx := context.Background() + + u, err := dbmodel.NewIdentity("alice@canonical.com") + c.Assert(err, qt.IsNil) + + alice := openfga.NewUser(u, j.OpenFGAClient) + alice.JimmAdmin = true + err = alice.SetControllerAccess(context.Background(), j.ResourceTag(), ofganames.AdministratorRelation) + c.Assert(err, qt.IsNil) + + ctl1 := dbmodel.Controller{ + Name: "test-controller", + AdminIdentityName: "admin", + AdminPassword: "5ecret", + PublicAddress: "example.com:443", + } + err = j.AddController(context.Background(), alice, &ctl1) + c.Assert(err, qt.IsNil) + + ctl2 := dbmodel.Controller{ + Name: "test-controller", + } + err = j.Database.GetController(ctx, &ctl2) + c.Assert(err, qt.IsNil) + c.Check(ctl2, qt.CmpEquals(cmpopts.EquateEmpty(), cmpopts.IgnoreTypes(dbmodel.CloudRegion{})), ctl1) + c.Check(ctl2.CloudRegion, qt.Equals, "default") + + cloud := dbmodel.Cloud{ + Name: "k8s", + } + err = j.Database.GetCloud(ctx, &cloud) + c.Assert(err, qt.IsNil) + c.Assert(cloud.Regions, qt.HasLen, 1) + c.Assert(cloud.Regions[0].Name, qt.Equals, "default") + c.Assert(cloud.Regions[0].Controllers, qt.HasLen, 1) + c.Assert(cloud.Regions[0].Controllers[0].Controller.Name, qt.Equals, ctl1.Name) +} + func TestAddControllerWithVault(t *testing.T) { c := qt.New(t) diff --git a/internal/jimm/export_test.go b/internal/jimm/export_test.go index 251233fa8..95f7fe4ef 100644 --- a/internal/jimm/export_test.go +++ b/internal/jimm/export_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm diff --git a/internal/jimm/group/group_test.go b/internal/jimm/group/group_test.go index 6e82b09ce..72a2a7681 100644 --- a/internal/jimm/group/group_test.go +++ b/internal/jimm/group/group_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package group_test diff --git a/internal/jimm/jimm_test.go b/internal/jimm/jimm_test.go index ec0dd3bce..2e9352607 100644 --- a/internal/jimm/jimm_test.go +++ b/internal/jimm/jimm_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm_test diff --git a/internal/jimm/model_cleanup_test.go b/internal/jimm/model_cleanup_test.go index b9f3299ab..08cfd432a 100644 --- a/internal/jimm/model_cleanup_test.go +++ b/internal/jimm/model_cleanup_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm_test diff --git a/internal/jimm/model_test.go b/internal/jimm/model_test.go index f6a27ed46..e5d22aad9 100644 --- a/internal/jimm/model_test.go +++ b/internal/jimm/model_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm_test diff --git a/internal/jimm/role/role_test.go b/internal/jimm/role/role_test.go index 786c179b4..fc0266b63 100644 --- a/internal/jimm/role/role_test.go +++ b/internal/jimm/role/role_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package role_test diff --git a/internal/jimm/watcher_test.go b/internal/jimm/watcher_test.go index 1f78fd1f5..edc99a3ad 100644 --- a/internal/jimm/watcher_test.go +++ b/internal/jimm/watcher_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm_test