Skip to content

Commit

Permalink
Merge pull request #1325 from alesstimec/implicit-cloud-for-add-model
Browse files Browse the repository at this point in the history
Allow adding a model with implicit cloud.
  • Loading branch information
alesstimec authored Aug 26, 2024
2 parents db18dbf + adcb70b commit c61fe72
Show file tree
Hide file tree
Showing 3 changed files with 246 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
12 changes: 5 additions & 7 deletions internal/jujuapi/modelmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -860,19 +860,17 @@ 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",
ownerTag: names.NewUserTag("[email protected]").String(),
cloudTag: names.NewCloudTag(jimmtest.TestCloudName).String(),
region: jimmtest.TestCloudRegionName,
}, {
about: "success - without a cloud tag",
name: "model-9",
ownerTag: names.NewUserTag("[email protected]").String(),
credentialTag: "cloudcred-" + jimmtest.TestCloudName + "[email protected]_cred",
}}

func (s *modelManagerSuite) TestCreateModel(c *gc.C) {
Expand Down

0 comments on commit c61fe72

Please sign in to comment.