From 6760ea5d60711263f127c19770c75b54c9fed615 Mon Sep 17 00:00:00 2001 From: SimoneDutto Date: Tue, 10 Dec 2024 10:09:04 +0100 Subject: [PATCH] fix model default order --- internal/jimm/model.go | 55 ++++++++++++++++--------------------- internal/jimm/model_test.go | 30 +++++++++++++++++--- 2 files changed, 49 insertions(+), 36 deletions(-) diff --git a/internal/jimm/model.go b/internal/jimm/model.go index e9ad44f99..edf173d10 100644 --- a/internal/jimm/model.go +++ b/internal/jimm/model.go @@ -245,6 +245,7 @@ func (b *modelBuilder) WithCloudRegion(region string) *modelBuilder { continue } region = r.Name + break } } // loop through all cloud regions @@ -577,22 +578,6 @@ func (j *JIMM) AddModel(ctx context.Context, user *openfga.User, args *ModelCrea return nil, errors.E(op, err) } - // fetch cloud defaults - // TODO(SimoneDutto): we should get the implicit cloud and then get the defaults. - if args.Cloud != (names.CloudTag{}) { - cloudDefaults := dbmodel.CloudDefaults{ - IdentityName: user.Name, - Cloud: dbmodel.Cloud{ - Name: args.Cloud.Id(), - }, - } - err = j.Database.CloudDefaults(ctx, &cloudDefaults) - if err != nil && errors.ErrorCode(err) != errors.CodeNotFound { - return nil, errors.E(op, "failed to fetch cloud defaults") - } - builder = builder.WithConfig(cloudDefaults.Defaults) - } - builder = builder.WithCloud(user, args.Cloud) if err := builder.Error(); err != nil { return nil, errors.E(op, err) @@ -602,6 +587,28 @@ func (j *JIMM) AddModel(ctx context.Context, user *openfga.User, args *ModelCrea if err := builder.Error(); err != nil { return nil, errors.E(op, err) } + // fetch cloud defaults + cloudDefaults := dbmodel.CloudDefaults{ + IdentityName: user.Name, + Cloud: *builder.cloud, + } + err = j.Database.CloudDefaults(ctx, &cloudDefaults) + if err != nil && errors.ErrorCode(err) != errors.CodeNotFound { + return nil, errors.E(op, "failed to fetch cloud defaults") + } + builder = builder.WithConfig(cloudDefaults.Defaults) + + // fetch cloud region defaults + cloudRegionDefaults := dbmodel.CloudDefaults{ + IdentityName: user.Name, + Cloud: *builder.cloud, + Region: builder.cloudRegion, + } + err = j.Database.CloudDefaults(ctx, &cloudRegionDefaults) + if err != nil && errors.ErrorCode(err) != errors.CodeNotFound { + return nil, errors.E(op, "failed to fetch cloud defaults") + } + builder = builder.WithConfig(cloudRegionDefaults.Defaults) // at this point we know which cloud will host the model and // we must check the user has add-model permission on the cloud @@ -613,22 +620,6 @@ func (j *JIMM) AddModel(ctx context.Context, user *openfga.User, args *ModelCrea return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized") } - // fetch cloud region defaults - if args.Cloud != (names.CloudTag{}) && builder.cloudRegion != "" { - cloudRegionDefaults := dbmodel.CloudDefaults{ - IdentityName: user.Name, - Cloud: dbmodel.Cloud{ - Name: args.Cloud.Id(), - }, - Region: builder.cloudRegion, - } - err = j.Database.CloudDefaults(ctx, &cloudRegionDefaults) - if err != nil && errors.ErrorCode(err) != errors.CodeNotFound { - return nil, errors.E(op, "failed to fetch cloud defaults") - } - builder = builder.WithConfig(cloudRegionDefaults.Defaults) - } - // last but not least, use the provided config values // overriding all defaults builder = builder.WithConfig(args.Config) diff --git a/internal/jimm/model_test.go b/internal/jimm/model_test.go index 39b82d886..3640059e0 100644 --- a/internal/jimm/model_test.go +++ b/internal/jimm/model_test.go @@ -373,6 +373,17 @@ cloud-credentials: owner: alice@canonical.com cloud: test-cloud auth-type: empty +cloud-defaults: +- user: alice@canonical.com + cloud: test-cloud + region: test-region-1 + defaults: + key1: value1 + key2: value2 +- user: alice@canonical.com + cloud: test-cloud + defaults: + key3: value3 controllers: - name: controller-1 uuid: 00000000-0000-0000-0000-0000-0000000000001 @@ -397,7 +408,11 @@ controllers: grantJIMMModelAdmin: func(_ context.Context, _ names.ModelTag) error { return nil }, - createModel: createModel(` + createModel: assertConfig(map[string]interface{}{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }, createModel(` uuid: 00000001-0000-0000-0000-0000-000000000001 status: status: started @@ -408,7 +423,7 @@ users: access: admin - user: bob access: read -`[1:]), +`[1:])), username: "alice@canonical.com", jimmAdmin: true, args: jujuparams.ModelCreateArgs{ @@ -884,10 +899,12 @@ cloud-defaults: defaults: key1: value1 key2: value2 + key4: value4 - user: alice@canonical.com cloud: test-cloud defaults: key3: value3 + key4: val5 cloud-credentials: - name: test-credential-1 owner: alice@canonical.com @@ -917,7 +934,12 @@ controllers: grantJIMMModelAdmin: func(_ context.Context, _ names.ModelTag) error { return nil }, - createModel: createModel(` + createModel: assertConfig(map[string]interface{}{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + "key4": "value4", + }, createModel(` uuid: 00000001-0000-0000-0000-0000-000000000001 status: status: started @@ -928,7 +950,7 @@ users: access: admin - user: bob access: read -`[1:]), +`[1:])), username: "alice@canonical.com", jimmAdmin: true, args: jujuparams.ModelCreateArgs{