Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow adding a model with implicit cloud. #1325

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Allow adding a model with implicit cloud. #1325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 46 additions & 13 deletions internal/jimm/model.go
View file
Open in desktop
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 {
alesstimec marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading