Skip to content

Commit

Permalink
Allow adding a model with implicit cloud.
Browse files Browse the repository at this point in the history
If the user does not specify which cloud to add the model to and only
one cloud is known to JIMM, we will select that cloud and continue instead
of returning an error.
  • Loading branch information
alesstimec committed Aug 26, 2024
1 parent db18dbf commit 749388e
Show file tree
Hide file tree
Showing 3 changed files with 241 additions and 28 deletions.
59 changes: 46 additions & 13 deletions internal/jimm/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,13 @@ func (a *ModelCreateArgs) FromJujuModelCreateArgs(args *jujuparams.ModelCreateAr
a.Name = args.Name
a.Config = args.Config
a.CloudRegion = args.CloudRegion
if args.CloudTag == "" {
return errors.E("no cloud specified for model; please specify one")
}
ct, err := names.ParseCloudTag(args.CloudTag)
if err != nil {
return errors.E(err, errors.CodeBadRequest)
if args.CloudTag != "" {
ct, err := names.ParseCloudTag(args.CloudTag)
if err != nil {
return errors.E(err, errors.CodeBadRequest)
}
a.Cloud = ct
}
a.Cloud = ct

if args.OwnerTag == "" {
return errors.E("owner tag not specified")
Expand Down Expand Up @@ -175,10 +174,17 @@ func (b *modelBuilder) WithConfig(cfg map[string]interface{}) *modelBuilder {
}

// WithCloud returns a builder with the specified cloud.
func (b *modelBuilder) WithCloud(cloud names.CloudTag) *modelBuilder {
func (b *modelBuilder) WithCloud(user *openfga.User, cloud names.CloudTag) *modelBuilder {
if b.err != nil {
return b
}

// if cloud was not specified then we try to determine if
// JIMM knows of only one cloud and use that one
if cloud.Id() == "" {
return b.withImplicitCloud(user)
}

c := dbmodel.Cloud{
Name: cloud.Id(),
}
Expand All @@ -192,6 +198,34 @@ func (b *modelBuilder) WithCloud(cloud names.CloudTag) *modelBuilder {
return b
}

// withImplicitCloud returns a builder with the only cloud known to JIMM. Should JIMM
// know of multiple clouds an error will be raised.
func (b *modelBuilder) withImplicitCloud(user *openfga.User) *modelBuilder {
if b.err != nil {
return b
}
var clouds []*dbmodel.Cloud
err := b.jimm.ForEachUserCloud(b.ctx, user, func(c *dbmodel.Cloud) error {
clouds = append(clouds, c)
return nil
})
if err != nil {
b.err = err
return b
}
if len(clouds) == 0 {
b.err = fmt.Errorf("no available clouds")
return b
}
if len(clouds) != 1 {
b.err = fmt.Errorf("no cloud specified for model; please specify one")
return b
}
b.cloud = clouds[0]

return b
}

// WithCloudRegion returns a builder with the specified cloud region.
func (b *modelBuilder) WithCloudRegion(region string) *modelBuilder {
if b.err != nil {
Expand Down Expand Up @@ -563,12 +597,11 @@ func (j *JIMM) AddModel(ctx context.Context, user *openfga.User, args *ModelCrea
builder = builder.WithConfig(cloudDefaults.Defaults)
}

if args.Cloud != (names.CloudTag{}) {
builder = builder.WithCloud(args.Cloud)
if err := builder.Error(); err != nil {
return nil, errors.E(op, err)
}
builder = builder.WithCloud(user, args.Cloud)
if err := builder.Error(); err != nil {
return nil, errors.E(op, err)
}

builder = builder.WithCloudRegion(args.CloudRegion)
if err := builder.Error(); err != nil {
return nil, errors.E(op, err)
Expand Down
203 changes: 195 additions & 8 deletions internal/jimm/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,6 @@ func TestModelCreateArgs(t *testing.T) {
CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice/test-credential-1").String(),
},
expectedError: "owner tag not specified",
}, {
about: "cloud tag not specified",
args: jujuparams.ModelCreateArgs{
Name: "test-model",
OwnerTag: names.NewUserTag("[email protected]").String(),
CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice/test-credential-1").String(),
},
expectedError: "no cloud specified for model; please specify one",
}}

opts := []cmp.Option{
Expand Down Expand Up @@ -886,6 +878,198 @@ users:
CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/[email protected]/test-credential-1").String(),
},
expectError: "unauthorized",
}, {
name: "CreateModelWithImplicitCloud",
env: `
clouds:
- name: test-cloud
type: test-provider
regions:
- name: test-region-1
users:
- user: [email protected]
access: add-model
user-defaults:
- user: [email protected]
defaults:
key4: value4
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
cloud-credentials:
- name: test-credential-1
owner: [email protected]
cloud: test-cloud
auth-type: empty
controllers:
- name: controller-1
uuid: 00000000-0000-0000-0000-0000-0000000000001
cloud: test-cloud
region: test-region-1
cloud-regions:
- cloud: test-cloud
region: test-region-1
priority: 0
- name: controller-2
uuid: 00000000-0000-0000-0000-0000-0000000000002
cloud: test-cloud
region: test-region-1
cloud-regions:
- cloud: test-cloud
region: test-region-1
priority: 2
`[1:],
updateCredential: func(_ context.Context, _ jujuparams.TaggedCredential) ([]jujuparams.UpdateCredentialModelResult, error) {
return nil, nil
},
grantJIMMModelAdmin: func(_ context.Context, _ names.ModelTag) error {
return nil
},
createModel: assertConfig(map[string]interface{}{
"key4": "value4",
}, createModel(`
uuid: 00000001-0000-0000-0000-0000-000000000001
status:
status: started
info: running a test
life: alive
users:
- user: [email protected]
access: admin
- user: bob
access: read
`[1:])),
username: "[email protected]",
jimmAdmin: true,
args: jujuparams.ModelCreateArgs{
Name: "test-model",
OwnerTag: names.NewUserTag("[email protected]").String(),
CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/[email protected]/test-credential-1").String(),
},
expectModel: dbmodel.Model{
Name: "test-model",
UUID: sql.NullString{
String: "00000001-0000-0000-0000-0000-000000000001",
Valid: true,
},
Owner: dbmodel.Identity{
Name: "[email protected]",
},
Controller: dbmodel.Controller{
Name: "controller-2",
UUID: "00000000-0000-0000-0000-0000-0000000000002",
CloudName: "test-cloud",
CloudRegion: "test-region-1",
},
CloudRegion: dbmodel.CloudRegion{
Cloud: dbmodel.Cloud{
Name: "test-cloud",
Type: "test-provider",
},
Name: "test-region-1",
},
CloudCredential: dbmodel.CloudCredential{
Name: "test-credential-1",
AuthType: "empty",
},
Life: state.Alive.String(),
Status: dbmodel.Status{
Status: "started",
Info: "running a test",
},
},
}, {
name: "CreateModelWithImplicitCloudAndMultipleClouds",
env: `
clouds:
- name: test-cloud
type: test-provider
regions:
- name: test-region-1
users:
- user: [email protected]
access: add-model
- name: test-cloud-2
type: test-provider-2
regions:
- name: test-region-2
users:
- user: [email protected]
access: add-model
user-defaults:
- user: [email protected]
defaults:
key4: value4
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
cloud-credentials:
- name: test-credential-1
owner: [email protected]
cloud: test-cloud
auth-type: empty
controllers:
- name: controller-1
uuid: 00000000-0000-0000-0000-0000-0000000000001
cloud: test-cloud
region: test-region-1
cloud-regions:
- cloud: test-cloud
region: test-region-1
priority: 0
- name: controller-2
uuid: 00000000-0000-0000-0000-0000-0000000000002
cloud: test-cloud
region: test-region-1
cloud-regions:
- cloud: test-cloud
region: test-region-1
priority: 2
`[1:],
updateCredential: func(_ context.Context, _ jujuparams.TaggedCredential) ([]jujuparams.UpdateCredentialModelResult, error) {
return nil, nil
},
grantJIMMModelAdmin: func(_ context.Context, _ names.ModelTag) error {
return nil
},
createModel: assertConfig(map[string]interface{}{
"key4": "value4",
}, createModel(`
uuid: 00000001-0000-0000-0000-0000-000000000001
status:
status: started
info: running a test
life: alive
users:
- user: [email protected]
access: admin
- user: bob
access: read
`[1:])),
username: "[email protected]",
jimmAdmin: true,
args: jujuparams.ModelCreateArgs{
Name: "test-model",
OwnerTag: names.NewUserTag("[email protected]").String(),
CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/[email protected]/test-credential-1").String(),
},
expectError: "no cloud specified for model; please specify one",
}}

func TestAddModel(t *testing.T) {
Expand Down Expand Up @@ -982,6 +1166,9 @@ func createModel(template string) func(context.Context, *jujuparams.ModelCreateA

func assertConfig(config map[string]interface{}, fnc func(context.Context, *jujuparams.ModelCreateArgs, *jujuparams.ModelInfo) error) func(context.Context, *jujuparams.ModelCreateArgs, *jujuparams.ModelInfo) error {
return func(ctx context.Context, args *jujuparams.ModelCreateArgs, mi *jujuparams.ModelInfo) error {
if args.CloudTag == "" {
return errors.E("cloud not specified")
}
if len(config) != len(args.Config) {
return errors.E(fmt.Sprintf("expected %d config settings, got %d", len(config), len(args.Config)))
}
Expand Down
7 changes: 0 additions & 7 deletions internal/jujuapi/modelmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -860,13 +860,6 @@ var createModelTests = []struct {
cloudTag: "not-a-cloud-tag",
credentialTag: "cloudcred-" + jimmtest.TestCloudName + "[email protected]_cred1",
expectError: `"not-a-cloud-tag" is not a valid tag \(bad request\)`,
}, {
about: "no cloud tag",
name: "model-8",
ownerTag: names.NewUserTag("[email protected]").String(),
cloudTag: "",
credentialTag: "cloudcred-" + jimmtest.TestCloudName + "[email protected]_cred1",
expectError: `no cloud specified for model; please specify one`,
}, {
about: "no credential tag selects unambigous creds",
name: "model-8",
Expand Down

0 comments on commit 749388e

Please sign in to comment.