feat: Implement SpaceBindingRequest controller main logic #848
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| package spacebindingrequest | ||
|
|
||
| import ( | ||
| "time" | ||
|
|
||
| toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" | ||
| "github.com/gofrs/uuid" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
|
@@ -34,3 +36,16 @@ func WithSpaceRole(spaceRole string) Option { | |
| spaceBindingRequest.Spec.SpaceRole = spaceRole | ||
| } | ||
| } | ||
|
|
||
| func WithDeletionTimestamp() Option { | ||
| return func(spaceBindingRequest *toolchainv1alpha1.SpaceBindingRequest) { | ||
| now := metav1.NewTime(time.Now()) | ||
| spaceBindingRequest.DeletionTimestamp = &now | ||
| } | ||
| } | ||
|
|
||
| func WithFinalizer() Option { | ||
| return func(spaceBindingRequest *toolchainv1alpha1.SpaceBindingRequest) { | ||
| spaceBindingRequest.Finalizers = append(spaceBindingRequest.Finalizers, toolchainv1alpha1.FinalizerName) | ||
|
There was a problem hiding this comment. what if the if spaceBindingRequest != nil {
spaceBindingRequest.Finalizers = append(spaceBindingRequest.Finalizers, toolchainv1alpha1.FinalizerName)
} else {
// Handle the case where the input spaceBindingRequest is nil, possibly with logging or an error return.
}There was a problem hiding this comment. Unless I'm missing something, it cannot be nil, since those Of course if somehow misused you could break them, but those are "only" used for unit testing, and we might want to modify all our There was a problem hiding this comment. same mistake from my part. I didn't see this is for testing purposes. Please ignore. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,12 @@ | ||
| package spacerequest | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "time" | ||
|
|
||
| toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" | ||
| "github.com/gofrs/uuid" | ||
| corev1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| ) | ||
|
|
@@ -61,3 +63,21 @@ func WithStatusNamespaceAccess(namespaceAccess toolchainv1alpha1.NamespaceAccess | |
| spaceRequest.Status.NamespaceAccess = []toolchainv1alpha1.NamespaceAccess{namespaceAccess} | ||
| } | ||
| } | ||
|
|
||
| func NewNamespace(spacename string) *corev1.Namespace { | ||
| labels := map[string]string{ | ||
| toolchainv1alpha1.TypeLabelKey: "tenant", | ||
| toolchainv1alpha1.ProviderLabelKey: toolchainv1alpha1.ProviderLabelValue, | ||
| } | ||
| if spacename != "nospace" { | ||
|
There was a problem hiding this comment. What happens if There was a problem hiding this comment. The function is used only for mocking namespace while unit testing. When is being called with "nospace" is done with the purpose of configuring (misconfiguring) the namespace, and verify the corner case in which a required label is missing, for example in this unit test: https://github.com/codeready-toolchain/host-operator/pull/848/files#diff-f15c73194e22112fd24e7e852200c395d2cddb8cb016fe76710f4c38bab782e1R232-R252 In short, I want the controller to throw this error here: https://github.com/codeready-toolchain/host-operator/pull/848/files#diff-e48d49741db80226348ae5ffd08100dd10afa2e83e5d18b2a94776dfdfb616f3R333-R335 Hope this makes sense. There was a problem hiding this comment. yes it makes sense. Sorry I missed the There was a problem hiding this comment. No worries! My pleasure getting more feedback/reviews and discussing alternative solutions! Thanks for checking! |
||
| labels[toolchainv1alpha1.SpaceLabelKey] = spacename | ||
| } | ||
| ns := &corev1.Namespace{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: fmt.Sprintf("%s-tenant", spacename), | ||
| Labels: labels, | ||
| }, | ||
| Status: corev1.NamespaceStatus{Phase: corev1.NamespaceActive}, | ||
| } | ||
| return ns | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not your code per se -- can we put "jane" in a const? Seems it's been used in many place, to avoid any typos in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, thanks for pointing this out, it doesn't look very clean, there are usually some strings that can be extracted to const or variable in our testing code, just didn't saw this aligned with the other unit tests for other controllers.
Or maybe we could put the string into a variable instead of const ? 🤔