diff --git a/pkg/apis/api.acorn.io/v1/region.go b/pkg/apis/api.acorn.io/v1/region.go index efe68603d..c54ba2451 100644 --- a/pkg/apis/api.acorn.io/v1/region.go +++ b/pkg/apis/api.acorn.io/v1/region.go @@ -9,6 +9,7 @@ const ( RegionConditionClusterReady = "ClusterReady" LocalRegion = "local" + AllRegions = "*" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/internal.acorn.io/v1/projectinstance.go b/pkg/apis/internal.acorn.io/v1/projectinstance.go index be5c33494..e17f0464b 100644 --- a/pkg/apis/internal.acorn.io/v1/projectinstance.go +++ b/pkg/apis/internal.acorn.io/v1/projectinstance.go @@ -20,8 +20,11 @@ type ProjectInstanceSpec struct { } type ProjectInstanceStatus struct { - Namespace string `json:"namespace,omitempty"` - DefaultRegion string `json:"defaultRegion,omitempty"` + Namespace string `json:"namespace,omitempty"` + DefaultRegion string `json:"defaultRegion,omitempty"` + // SupportedRegions on the status field should be an explicit list of supported regions. + // That is, if the user specifies "*" for supported regions, then the status value should be the list of all regions. + // This is to avoid having to make another call to explicitly list all regions. SupportedRegions []string `json:"supportedRegions,omitempty"` } @@ -44,7 +47,7 @@ func (in *ProjectInstance) GetSupportedRegions() []string { func (in *ProjectInstance) SetDefaultRegion(region string) { if in.Spec.DefaultRegion == "" && len(in.Spec.SupportedRegions) == 0 { in.Status.DefaultRegion = region - in.Status.SupportedRegions = []string{region} + in.Status.SupportedRegions = []string{"*"} } else { // Set the status values to the provided spec values. // The idea here is that internally, we only need to check the status values. diff --git a/pkg/client/project.go b/pkg/client/project.go index d71e580ed..d4b2696db 100644 --- a/pkg/client/project.go +++ b/pkg/client/project.go @@ -37,7 +37,7 @@ func (c *DefaultClient) ProjectCreate(ctx context.Context, name, defaultRegion s } if defaultRegion != "" { project.Spec.DefaultRegion = defaultRegion - if !slices.Contains(supportedRegions, defaultRegion) { + if !slices.Contains(supportedRegions, defaultRegion) && !slices.Contains(supportedRegions, apiv1.AllRegions) { supportedRegions = append([]string{defaultRegion}, supportedRegions...) } } @@ -57,7 +57,7 @@ func (c *DefaultClient) ProjectUpdate(ctx context.Context, project *apiv1.Projec project.Spec.SupportedRegions = supportedRegions } } - if project.Spec.DefaultRegion != "" && !slices.Contains(project.Spec.SupportedRegions, project.Spec.DefaultRegion) { + if project.Spec.DefaultRegion != "" && !slices.Contains(project.Spec.SupportedRegions, project.Spec.DefaultRegion) && !slices.Contains(project.Spec.SupportedRegions, apiv1.AllRegions) { project.Spec.SupportedRegions = append(project.Spec.SupportedRegions, project.Spec.DefaultRegion) } diff --git a/pkg/openapi/generated/openapi_generated.go b/pkg/openapi/generated/openapi_generated.go index c66dd629e..7152a633d 100644 --- a/pkg/openapi/generated/openapi_generated.go +++ b/pkg/openapi/generated/openapi_generated.go @@ -9437,7 +9437,8 @@ func schema_pkg_apis_internalacornio_v1_ProjectInstanceStatus(ref common.Referen }, "supportedRegions": { SchemaProps: spec.SchemaProps{ - Type: []string{"array"}, + Description: "SupportedRegions on the status field should be an explicit list of supported regions. That is, if the user specifies \"*\" for supported regions, then the status value should be the list of all regions. This is to avoid having to make another call to explicitly list all regions.", + Type: []string{"array"}, Items: &spec.SchemaOrArray{ Schema: &spec.Schema{ SchemaProps: spec.SchemaProps{ diff --git a/pkg/project/handlers.go b/pkg/project/handlers.go index fb14bd841..caa5701e9 100644 --- a/pkg/project/handlers.go +++ b/pkg/project/handlers.go @@ -12,11 +12,20 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" klabels "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" + "k8s.io/utils/strings/slices" kclient "sigs.k8s.io/controller-runtime/pkg/client" ) func SetProjectSupportedRegions(req router.Request, resp router.Response) error { - req.Object.(*v1.ProjectInstance).SetDefaultRegion(apiv1.LocalRegion) + project := req.Object.(*v1.ProjectInstance) + project.SetDefaultRegion(apiv1.LocalRegion) + if slices.Contains(project.Status.SupportedRegions, apiv1.AllRegions) { + // If the project supports all regions, then ensure the default region and the local region are supported regions. + project.Status.SupportedRegions = []string{project.Status.DefaultRegion} + if project.Status.DefaultRegion != apiv1.LocalRegion { + project.Status.SupportedRegions = append(project.Status.SupportedRegions, apiv1.LocalRegion) + } + } resp.Objects(req.Object) return nil diff --git a/pkg/project/handlers_test.go b/pkg/project/handlers_test.go index cf517a9c4..452c9d265 100644 --- a/pkg/project/handlers_test.go +++ b/pkg/project/handlers_test.go @@ -18,6 +18,7 @@ func TestSetProjectSupportedRegions(t *testing.T) { tester.DefaultTest(t, scheme.Scheme, "testdata/setsupportedregions/no-default", SetProjectSupportedRegions) tester.DefaultTest(t, scheme.Scheme, "testdata/setsupportedregions/with-supported-regions", SetProjectSupportedRegions) tester.DefaultTest(t, scheme.Scheme, "testdata/setsupportedregions/with-default-and-supported", SetProjectSupportedRegions) + tester.DefaultTest(t, scheme.Scheme, "testdata/setsupportedregions/all-supported-regions-with-default", SetProjectSupportedRegions) } func TestCreateNamespace(t *testing.T) { diff --git a/pkg/project/testdata/setsupportedregions/all-supported-regions-with-default/expected.golden b/pkg/project/testdata/setsupportedregions/all-supported-regions-with-default/expected.golden new file mode 100644 index 000000000..1b506435f --- /dev/null +++ b/pkg/project/testdata/setsupportedregions/all-supported-regions-with-default/expected.golden @@ -0,0 +1,15 @@ +`apiVersion: internal.acorn.io/v1 +kind: ProjectInstance +metadata: + creationTimestamp: null + name: acorn +spec: + defaultRegion: other-region + supportedRegions: + - '*' +status: + defaultRegion: other-region + supportedRegions: + - other-region + - local +` diff --git a/pkg/project/testdata/setsupportedregions/all-supported-regions-with-default/input.yaml b/pkg/project/testdata/setsupportedregions/all-supported-regions-with-default/input.yaml new file mode 100644 index 000000000..4f521e86d --- /dev/null +++ b/pkg/project/testdata/setsupportedregions/all-supported-regions-with-default/input.yaml @@ -0,0 +1,8 @@ +kind: ProjectInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: acorn +spec: + defaultRegion: other-region + supportedRegions: + - "*" \ No newline at end of file diff --git a/pkg/server/registry/apigroups/acorn/projects/validator.go b/pkg/server/registry/apigroups/acorn/projects/validator.go index 43ecb5aa3..437dec0d6 100644 --- a/pkg/server/registry/apigroups/acorn/projects/validator.go +++ b/pkg/server/registry/apigroups/acorn/projects/validator.go @@ -26,46 +26,42 @@ func (v *Validator) Validate(_ context.Context, obj runtime.Object) field.ErrorL var result field.ErrorList project := obj.(*apiv1.Project) - if project.Spec.DefaultRegion != "" && !slices.Contains(project.Spec.SupportedRegions, project.Spec.DefaultRegion) { + if project.Spec.DefaultRegion != "" && !slices.Contains(project.Spec.SupportedRegions, project.Spec.DefaultRegion) && !slices.Contains(project.Spec.SupportedRegions, apiv1.AllRegions) { return append(result, field.Invalid(field.NewPath("spec", "defaultRegion"), project.Spec.DefaultRegion, "default region is not in the supported regions list")) } return nil } -func (v *Validator) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { +func (v *Validator) ValidateUpdate(ctx context.Context, newObj, _ runtime.Object) field.ErrorList { // Ensure that default region and supported regions are valid. - if err := v.Validate(ctx, obj); err != nil { + if err := v.Validate(ctx, newObj); err != nil { return err } - // If the user is removing a supported region, ensure that there are no apps in that region. - oldProject, newProject := old.(*apiv1.Project), obj.(*apiv1.Project) - var removedRegions []string - for _, region := range oldProject.Status.SupportedRegions { - if !slices.Contains(newProject.Status.SupportedRegions, region) { - removedRegions = append(removedRegions, region) - } - } - - if len(removedRegions) > 0 { - return v.ensureNoObjectsExistInRegions(ctx, newProject.Name, newProject.Status.SupportedRegions, removedRegions, &apiv1.AppList{}, &apiv1.VolumeList{}) + newProject := newObj.(*apiv1.Project) + // If there are no supported regions given by the user (and the above validate call passed) or the user explicitly + // allowed all regions, then the project supports all regions. + if len(newProject.Spec.SupportedRegions) == 0 || slices.Contains(newProject.Spec.SupportedRegions, apiv1.AllRegions) { + return nil } - return nil + // If the user is removing a supported region, ensure that there are no apps in that region. + return v.ensureNoObjectsExistOutsideOfRegions(ctx, newProject.Name, newProject.Spec.SupportedRegions, &apiv1.AppList{}, &apiv1.VolumeList{}) } -func (v *Validator) ensureNoObjectsExistInRegions(ctx context.Context, namespace string, regions, removedRegions []string, objList ...kclient.ObjectList) field.ErrorList { +func (v *Validator) ensureNoObjectsExistOutsideOfRegions(ctx context.Context, namespace string, regions []string, objList ...kclient.ObjectList) field.ErrorList { var result field.ErrorList for _, obj := range objList { - var inRemovedRegion []string + var removedRegions, inRemovedRegion []string if err := v.Client.List(ctx, obj, kclient.InNamespace(namespace)); err != nil { return field.ErrorList{field.InternalError(field.NewPath("spec", "supportedRegions"), err)} } if err := meta.EachListItem(obj, func(object runtime.Object) error { regionObject, ok := object.(regionNamer) - if ok && slices.Contains(removedRegions, regionObject.GetRegion()) { + if ok && !slices.Contains(regions, regionObject.GetRegion()) { + removedRegions = append(removedRegions, regionObject.GetRegion()) inRemovedRegion = append(inRemovedRegion, regionObject.GetName()) } return nil diff --git a/pkg/server/registry/apigroups/acorn/projects/validator_test.go b/pkg/server/registry/apigroups/acorn/projects/validator_test.go index 67fdb4d17..695fd9461 100644 --- a/pkg/server/registry/apigroups/acorn/projects/validator_test.go +++ b/pkg/server/registry/apigroups/acorn/projects/validator_test.go @@ -48,7 +48,7 @@ func TestProjectCreateValidation(t *testing.T) { }, }, { - name: "Create project with default that is not supported", + name: "Create project with default and no supported regions should fail", wantError: true, project: apiv1.Project{ Spec: v1.ProjectInstanceSpec{ @@ -57,6 +57,16 @@ func TestProjectCreateValidation(t *testing.T) { }, }, }, + { + name: "Create project with default that is not supported", + wantError: true, + project: apiv1.Project{ + Spec: v1.ProjectInstanceSpec{ + DefaultRegion: "acorn-test-region", + SupportedRegions: []string{"acorn-test-other"}, + }, + }, + }, { name: "Create project with supported region that does not exist is valid", project: apiv1.Project{ @@ -74,6 +84,15 @@ func TestProjectCreateValidation(t *testing.T) { }, }, }, + { + name: "Create project with default region supporting all regions", + project: apiv1.Project{ + Spec: v1.ProjectInstanceSpec{ + DefaultRegion: "acorn-test-region", + SupportedRegions: []string{apiv1.AllRegions}, + }, + }, + }, } for _, tt := range tests { @@ -128,6 +147,7 @@ func TestProjectUpdateValidation(t *testing.T) { SupportedRegions: []string{"my-region"}, }, }, + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), }, { name: "Update project to have default region and non-existent supported regions", @@ -148,6 +168,7 @@ func TestProjectUpdateValidation(t *testing.T) { SupportedRegions: []string{"my-region", "dne-region"}, }, }, + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), }, { name: "Remove default region as supported region", @@ -163,7 +184,7 @@ func TestProjectUpdateValidation(t *testing.T) { }, }, { - name: "Update project remove a supported region, no apps", + name: "Update project to remove a supported region, no apps", oldProject: apiv1.Project{ ObjectMeta: metav1.ObjectMeta{ Name: "my-project", @@ -185,7 +206,7 @@ func TestProjectUpdateValidation(t *testing.T) { client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), }, { - name: "Update project remove a supported region, no apps in project", + name: "Update project to remove a supported region, no apps in project", oldProject: apiv1.Project{ ObjectMeta: metav1.ObjectMeta{ Name: "my-project", @@ -221,7 +242,7 @@ func TestProjectUpdateValidation(t *testing.T) { ).Build(), }, { - name: "Update project remove a supported region, no apps in removed region", + name: "Update project to remove a supported region, no apps in removed region", oldProject: apiv1.Project{ ObjectMeta: metav1.ObjectMeta{ Name: "my-project", @@ -257,7 +278,7 @@ func TestProjectUpdateValidation(t *testing.T) { ).Build(), }, { - name: "Update project remove a supported region with apps in removed region", + name: "Update project to remove a supported region with apps in removed region", wantError: true, oldProject: apiv1.Project{ ObjectMeta: metav1.ObjectMeta{ @@ -298,7 +319,7 @@ func TestProjectUpdateValidation(t *testing.T) { ).Build(), }, { - name: "Update project remove a supported region with volumes in removed region", + name: "Update project to remove a supported region with volumes in removed region", wantError: true, oldProject: apiv1.Project{ ObjectMeta: metav1.ObjectMeta{ @@ -339,7 +360,7 @@ func TestProjectUpdateValidation(t *testing.T) { ).Build(), }, { - name: "Update project remove a supported region with apps defaulted to removed region", + name: "Update project to remove a supported region with apps defaulted to removed region", wantError: true, oldProject: apiv1.Project{ ObjectMeta: metav1.ObjectMeta{ @@ -362,6 +383,10 @@ func TestProjectUpdateValidation(t *testing.T) { DefaultRegion: "my-region", SupportedRegions: []string{"my-region"}, }, + Status: v1.ProjectInstanceStatus{ + DefaultRegion: "my-region", + SupportedRegions: []string{"my-region", "my-other-region"}, + }, }, client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithLists( &apiv1.AppList{ @@ -381,6 +406,144 @@ func TestProjectUpdateValidation(t *testing.T) { }, ).Build(), }, + { + name: "Update project to remove a supported region with apps using removed region", + wantError: true, + oldProject: apiv1.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-project", + }, + Spec: v1.ProjectInstanceSpec{ + DefaultRegion: "my-region", + SupportedRegions: []string{"my-region", "my-other-region"}, + }, + Status: v1.ProjectInstanceStatus{ + DefaultRegion: "my-region", + SupportedRegions: []string{"my-region", "my-other-region"}, + }, + }, + newProject: apiv1.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-project", + }, + Spec: v1.ProjectInstanceSpec{ + DefaultRegion: "my-region", + SupportedRegions: []string{"my-region"}, + }, + Status: v1.ProjectInstanceStatus{ + DefaultRegion: "my-region", + SupportedRegions: []string{"my-region", "my-other-region"}, + }, + }, + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithLists( + &apiv1.AppList{ + Items: []apiv1.App{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-app", + Namespace: "my-project", + }, + Spec: v1.AppInstanceSpec{ + Region: "my-other-region", + }, + }, + }, + }, + ).Build(), + }, + { + name: "Update project to change default region and still allow all regions", + oldProject: apiv1.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-project", + }, + Spec: v1.ProjectInstanceSpec{ + DefaultRegion: "my-region", + SupportedRegions: []string{apiv1.AllRegions}, + }, + Status: v1.ProjectInstanceStatus{ + DefaultRegion: "my-region", + }, + }, + newProject: apiv1.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-project", + }, + Spec: v1.ProjectInstanceSpec{ + DefaultRegion: "my-other-region", + SupportedRegions: []string{apiv1.AllRegions}, + }, + Status: v1.ProjectInstanceStatus{ + DefaultRegion: "my-region", + }, + }, + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithLists( + &apiv1.AppList{ + Items: []apiv1.App{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-other-app", + Namespace: "my-project", + }, + Spec: v1.AppInstanceSpec{ + Region: "my-other-region", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-app", + Namespace: "my-project", + }, + Spec: v1.AppInstanceSpec{ + Region: "my-region", + }, + }, + }, + }, + ).Build(), + }, + { + name: "Update project to remove supported regions, but app exists in removed region", + wantError: true, + oldProject: apiv1.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-project", + }, + Spec: v1.ProjectInstanceSpec{ + DefaultRegion: "my-region", + }, + Status: v1.ProjectInstanceStatus{ + DefaultRegion: "my-region", + }, + }, + newProject: apiv1.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-project", + }, + Spec: v1.ProjectInstanceSpec{ + DefaultRegion: "my-region", + SupportedRegions: []string{"my-region"}, + }, + Status: v1.ProjectInstanceStatus{ + DefaultRegion: "my-region", + }, + }, + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithLists( + &apiv1.AppList{ + Items: []apiv1.App{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-app", + Namespace: "my-project", + }, + Spec: v1.AppInstanceSpec{ + Region: "my-other-region", + }, + }, + }, + }, + ).Build(), + }, } for _, tt := range tests {