Skip to content

Commit

Permalink
fix model default order (#1487)
Browse files Browse the repository at this point in the history
  • Loading branch information
SimoneDutto authored Dec 16, 2024
1 parent 486ff61 commit fd629e9
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 36 deletions.
55 changes: 23 additions & 32 deletions internal/jimm/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ func (b *modelBuilder) WithCloudRegion(region string) *modelBuilder {
continue
}
region = r.Name
break
}
}
// loop through all cloud regions
Expand Down Expand Up @@ -579,22 +580,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)
Expand All @@ -604,6 +589,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
Expand All @@ -615,22 +622,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)
Expand Down
30 changes: 26 additions & 4 deletions internal/jimm/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,17 @@ cloud-credentials:
owner: [email protected]
cloud: test-cloud
auth-type: empty
cloud-defaults:
- user: [email protected]
cloud: test-cloud
region: test-region-1
defaults:
key1: value1
key2: value2
- user: [email protected]
cloud: test-cloud
defaults:
key3: value3
controllers:
- name: controller-1
uuid: 00000000-0000-0000-0000-0000-0000000000001
Expand All @@ -391,7 +402,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
Expand All @@ -402,7 +417,7 @@ users:
access: admin
- user: bob
access: read
`[1:]),
`[1:])),
username: "[email protected]",
jimmAdmin: true,
args: jujuparams.ModelCreateArgs{
Expand Down Expand Up @@ -870,10 +885,12 @@ cloud-defaults:
defaults:
key1: value1
key2: value2
key4: value4
- user: [email protected]
cloud: test-cloud
defaults:
key3: value3
key4: val5
cloud-credentials:
- name: test-credential-1
owner: [email protected]
Expand Down Expand Up @@ -903,7 +920,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
Expand All @@ -914,7 +936,7 @@ users:
access: admin
- user: bob
access: read
`[1:]),
`[1:])),
username: "[email protected]",
jimmAdmin: true,
args: jujuparams.ModelCreateArgs{
Expand Down

0 comments on commit fd629e9

Please sign in to comment.