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.

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

[DRAFT] Allow for disable inheritace when creating spaces #923

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ spec:
spec:
description: SpaceRequestSpec defines the desired state of Space
properties:
disableInheritance:
description: "DisableInheritance indicates whether or not SpaceBindings
from the parent-spaces are automatically inherited to all sub-spaces
in the tree. \n Set to True to disable SpaceBinding inheritance
from the parent-spaces. Default is False."
type: boolean
targetClusterRoles:
description: TargetClusterRoles one or more label keys that define
a set of clusters where the Space can be provisioned. The target
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/toolchain.dev.openshift.com_spaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ spec:
spec:
description: SpaceSpec defines the desired state of Space
properties:
disableInheritance:
description: "DisableInheritance indicates whether or not SpaceBindings
from the parent-spaces are automatically inherited to all sub-spaces
in the tree. \n Set to True to disable SpaceBinding inheritance
from the parent-spaces. Default is False."
type: boolean
parentSpace:
description: "ParentSpace holds the name of the context (Space) from
which this space was created (requested), enabling hierarchy relationships
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,18 @@ spec:
description: Defines the flag that determines whether
to deploy the Webhook
type: boolean
secret:
description: Defines all secrets related to webhook configuration
properties:
ref:
description: Reference is the name of the secret resource
to look up
type: string
virtualMachineAccessKey:
description: The key in the secret values map that
contains a comma-separated list of SSH keys
type: string
type: object
type: object
type: object
specificPerMemberCluster:
Expand Down Expand Up @@ -759,6 +771,19 @@ spec:
description: Defines the flag that determines whether
to deploy the Webhook
type: boolean
secret:
description: Defines all secrets related to webhook
configuration
properties:
ref:
description: Reference is the name of the secret
resource to look up
type: string
virtualMachineAccessKey:
description: The key in the secret values map that
contains a comma-separated list of SSH keys
type: string
type: object
type: object
type: object
description: A map of cluster-specific member operator configurations
Expand Down
37 changes: 35 additions & 2 deletions controllers/spacerequest/spacerequest_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ func TestCreateSpaceRequest(t *testing.T) {
t.Run("success", func(t *testing.T) {
sr := spacerequesttest.NewSpaceRequest("jane", srNamespace.GetName(),
spacerequesttest.WithTierName("appstudio"),
spacerequesttest.WithTargetClusterRoles(srClusterRoles))
spacerequesttest.WithTargetClusterRoles(srClusterRoles),
spacerequesttest.WithDisableInheritance(false))

t.Run("subSpace doesn't exists it should be created", func(t *testing.T) {
// given
Expand All @@ -59,12 +60,44 @@ func TestCreateSpaceRequest(t *testing.T) {
spacerequesttest.AssertThatSpaceRequest(t, srNamespace.Name, sr.GetName(), member1.Client).
HasSpecTargetClusterRoles(srClusterRoles).
HasSpecTierName("appstudio").
HasDisableInheritance(false).
HasConditions(spacetest.Provisioning()).
HasFinalizer()
// there should be 1 subSpace that was created from the spacerequest
spacetest.AssertThatSubSpace(t, hostClient, sr, parentSpace).
HasTier("appstudio").
HasSpecTargetClusterRoles(srClusterRoles)
HasSpecTargetClusterRoles(srClusterRoles).
HasDisableInheritance(false)
})

t.Run("subSpace created with disableInheritance", func(t *testing.T) {
sr := spacerequesttest.NewSpaceRequest("jane", srNamespace.GetName(),
spacerequesttest.WithTierName("appstudio"),
spacerequesttest.WithTargetClusterRoles(srClusterRoles),
spacerequesttest.WithDisableInheritance(true))

// given
member1 := NewMemberClusterWithClient(test.NewFakeClient(t, sr, srNamespace), "member-1", corev1.ConditionTrue)
hostClient := test.NewFakeClient(t, appstudioTier, parentSpace)
ctrl := newReconciler(t, hostClient, member1)

// when
_, err = ctrl.Reconcile(context.TODO(), requestFor(sr))

// then
require.NoError(t, err)
// spacerequest exists with expected cluster roles and finalizer
spacerequesttest.AssertThatSpaceRequest(t, srNamespace.Name, sr.GetName(), member1.Client).
HasSpecTargetClusterRoles(srClusterRoles).
HasSpecTierName("appstudio").
HasDisableInheritance(true).
HasConditions(spacetest.Provisioning()).
HasFinalizer()
// there should be 1 subSpace that was created from the spacerequest
spacetest.AssertThatSubSpace(t, hostClient, sr, parentSpace).
HasTier("appstudio").
HasSpecTargetClusterRoles(srClusterRoles).
HasDisableInheritance(true)
})

t.Run("subSpace exists but is not ready yet", func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion controllers/usersignup/spacebinding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestNewSpaceBinding(t *testing.T) {
// given
userSignup := commonsignup.NewUserSignup()
nsTemplateTier := tiertest.NewNSTemplateTier("advanced", "dev", "stage", "extra")
space := NewSpace(userSignup, test.MemberClusterName, "smith", nsTemplateTier.Name)
space := NewSpace(userSignup, test.MemberClusterName, "smith", nsTemplateTier.Name, false)
mur := newMasterUserRecord(userSignup, test.MemberClusterName, "deactivate90", "johny")

// when
Expand Down
2 changes: 1 addition & 1 deletion controllers/usersignup/usersignup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ func (r *Reconciler) ensureSpace(
}
tCluster := targetCluster(mur.Spec.UserAccounts[0].TargetCluster)

space = spaceutil.NewSpace(userSignup, tCluster.getClusterName(), mur.Name, spaceTier.Name)
space = spaceutil.NewSpace(userSignup, tCluster.getClusterName(), mur.Name, spaceTier.Name, false)

err = r.Client.Create(ctx, space)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions controllers/usersignup/usersignup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1584,7 +1584,7 @@ func TestUserSignupMUROrSpaceOrSpaceBindingCreateFails(t *testing.T) {
mur := newMasterUserRecord(userSignup, "member1", deactivate30Tier.Name, "foo")
mur.Labels = map[string]string{toolchainv1alpha1.MasterUserRecordOwnerLabelKey: userSignup.Name}

space := NewSpace(userSignup, "member1", "foo", "base")
space := NewSpace(userSignup, "member1", "foo", "base", false)

ready := NewGetMemberClusters(NewMemberClusterWithTenantRole(t, "member1", corev1.ConditionTrue))
initObjs := []runtime.Object{userSignup, baseNSTemplateTier, deactivate30Tier}
Expand Down Expand Up @@ -1868,7 +1868,7 @@ func TestUserSignupWithExistingMUROK(t *testing.T) {
},
}

space := NewSpace(userSignup, "member1", "foo", "base")
space := NewSpace(userSignup, "member1", "foo", "base", false)

spacebinding := spacebindingtest.NewSpaceBinding("foo", "foo", "admin", userSignup.Name)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from: base
parameters:
- name: MEMORY_LIMIT
value: "16Gi"
value: "32Gi"
- name: MEMORY_REQUEST
value: "16Gi"
value: "32Gi"
- name: IDLER_TIMEOUT_SECONDS
value: 0
12 changes: 6 additions & 6 deletions deploy/templates/nstemplatetiers/base/cluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ objects:
spec:
quota:
hard:
limits.cpu: 20000m
limits.cpu: 40000m
limits.memory: ${MEMORY_LIMIT}
limits.ephemeral-storage: 7Gi
requests.cpu: 1750m
limits.ephemeral-storage: 15Gi
requests.cpu: 6000m
requests.memory: ${MEMORY_REQUEST}
requests.storage: 40Gi
requests.ephemeral-storage: 7Gi
requests.ephemeral-storage: 15Gi
count/persistentvolumeclaims: "5"
selector:
annotations: null
Expand Down Expand Up @@ -153,6 +153,6 @@ parameters:
# 12 hours
value: "43200"
- name: MEMORY_LIMIT
value: "7Gi"
value: "28Gi"
- name: MEMORY_REQUEST
value: "7Gi"
value: "28Gi"
2 changes: 1 addition & 1 deletion deploy/templates/nstemplatetiers/base/ns_dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ objects:
- type: "Container"
default:
cpu: 1000m
memory: 750Mi
memory: 1024Mi
defaultRequest:
cpu: 10m
memory: 64Mi
Expand Down
2 changes: 1 addition & 1 deletion deploy/templates/nstemplatetiers/base/ns_stage.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ objects:
- type: "Container"
default:
cpu: 1000m
memory: 750Mi
memory: 1024Mi
defaultRequest:
cpu: 10m
memory: 64Mi
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from: base
parameters:
- name: MEMORY_LIMIT
value: "16Gi"
value: "32Gi"
- name: MEMORY_REQUEST
value: "16Gi"
value: "32Gi"
4 changes: 3 additions & 1 deletion pkg/space/space.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
)

// NewSpace creates a space CR for a UserSignup object.
func NewSpace(userSignup *toolchainv1alpha1.UserSignup, targetClusterName string, compliantUserName, tier string) *toolchainv1alpha1.Space {
func NewSpace(userSignup *toolchainv1alpha1.UserSignup, targetClusterName string, compliantUserName, tier string, disableInheritance bool) *toolchainv1alpha1.Space {
labels := map[string]string{
toolchainv1alpha1.SpaceCreatorLabelKey: userSignup.Name,
}
Expand All @@ -22,6 +22,7 @@
TargetCluster: targetClusterName,
TargetClusterRoles: []string{cluster.RoleLabel(cluster.Tenant)}, // by default usersignups should be provisioned to tenant clusters
TierName: tier,
DisableInheritance: disableInheritance,

Check failure on line 25 in pkg/space/space.go

View workflow job for this annotation

GitHub Actions / Test with Coverage

unknown field 'DisableInheritance' in struct literal of type v1alpha1.SpaceSpec

Check failure on line 25 in pkg/space/space.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint

unknown field 'DisableInheritance' in struct literal of type v1alpha1.SpaceSpec
},
}
return space
Expand All @@ -45,6 +46,7 @@
TargetClusterRoles: spaceRequest.Spec.TargetClusterRoles,
TierName: spaceRequest.Spec.TierName,
ParentSpace: parentSpace.GetName(),
DisableInheritance: spaceRequest.Spec.DisableInheritance,

Check failure on line 49 in pkg/space/space.go

View workflow job for this annotation

GitHub Actions / Test with Coverage

unknown field 'DisableInheritance' in struct literal of type v1alpha1.SpaceSpec

Check failure on line 49 in pkg/space/space.go

View workflow job for this annotation

GitHub Actions / Test with Coverage

spaceRequest.Spec.DisableInheritance undefined (type v1alpha1.SpaceRequestSpec has no field or method DisableInheritance)

Check failure on line 49 in pkg/space/space.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint

unknown field 'DisableInheritance' in struct literal of type v1alpha1.SpaceSpec

Check failure on line 49 in pkg/space/space.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint

spaceRequest.Spec.DisableInheritance undefined (type v1alpha1.SpaceRequestSpec has no field or method DisableInheritance)) (typecheck)
},
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/space/space_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestNewSpace(t *testing.T) {
userSignup := commonsignup.NewUserSignup()

// when
space := NewSpace(userSignup, test.MemberClusterName, "johny", "advanced")
space := NewSpace(userSignup, test.MemberClusterName, "johny", "advanced", false)

// then
expectedSpace := spacetest.NewSpace(test.HostOperatorNs, "johny",
Expand Down
6 changes: 6 additions & 0 deletions test/spacerequest/spacerequest.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package spacerequest

Check failure on line 1 in test/spacerequest/spacerequest.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint

: # github.com/codeready-toolchain/host-operator/test/spacerequest

import (
"fmt"
Expand Down Expand Up @@ -39,6 +39,12 @@
}
}

func WithDisableInheritance(disableInheritance bool) Option {
return func(spaceRequest *toolchainv1alpha1.SpaceRequest) {
spaceRequest.Spec.DisableInheritance = disableInheritance

Check failure on line 44 in test/spacerequest/spacerequest.go

View workflow job for this annotation

GitHub Actions / Test with Coverage

spaceRequest.Spec.DisableInheritance undefined (type v1alpha1.SpaceRequestSpec has no field or method DisableInheritance)

Check failure on line 44 in test/spacerequest/spacerequest.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint

spaceRequest.Spec.DisableInheritance undefined (type v1alpha1.SpaceRequestSpec has no field or method DisableInheritance)

Check failure on line 44 in test/spacerequest/spacerequest.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint

spaceRequest.Spec.DisableInheritance undefined (type v1alpha1.SpaceRequestSpec has no field or method DisableInheritance)
}
}

func WithDeletionTimestamp() Option {
return func(spaceRequest *toolchainv1alpha1.SpaceRequest) {
now := metav1.NewTime(time.Now())
Expand Down
7 changes: 7 additions & 0 deletions test/spacerequest/spacerequest_assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@
return a
}

func (a *Assertion) HasDisableInheritance(disableInheritance bool) *Assertion {
err := a.loadResource()
require.NoError(a.t, err)
assert.Equal(a.t, disableInheritance, a.spaceRequest.Spec.DisableInheritance)

Check failure on line 125 in test/spacerequest/spacerequest_assertions.go

View workflow job for this annotation

GitHub Actions / Test with Coverage

a.spaceRequest.Spec.DisableInheritance undefined (type v1alpha1.SpaceRequestSpec has no field or method DisableInheritance)

Check failure on line 125 in test/spacerequest/spacerequest_assertions.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint

a.spaceRequest.Spec.DisableInheritance undefined (type v1alpha1.SpaceRequestSpec has no field or method DisableInheritance) (typecheck)

Check failure on line 125 in test/spacerequest/spacerequest_assertions.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint

a.spaceRequest.Spec.DisableInheritance undefined (type v1alpha1.SpaceRequestSpec has no field or method DisableInheritance)) (typecheck)
return a
}

func (a *Assertion) HasTargetClusterURL(targetCluster string) *Assertion {
err := a.loadResource()
require.NoError(a.t, err)
Expand Down
Loading