Skip to content

Commit

Permalink
Get Secrets for Service Manager (#655)
Browse files Browse the repository at this point in the history
* Add interface for cluster object provider

* Interface segregation

* Add context param to interfaces

* Add NamespaceProvider

* Add SecretProvider

* Add ServiceInstanceProvider

* tweaks - use pointers, name changes

* Include ServiceInstanceProvider in SecretProvider

* Get secrets from secret ref in SI

* Fix provider interface

* Fix constructor

* Add NamespaceProvider unit tests

* Fix ServiceInstanceProvider constructor

* Fix secret reference filter

* Add unit tests for service instance provider

* Fix secrets fetching

* Log warning when SecretProvider does not find btp operator secrets

* Add SecretProvider unit tests

* Apply suggestions from code review
  • Loading branch information
szwedm committed May 7, 2024
1 parent 551232a commit 21d3779
Show file tree
Hide file tree
Showing 13 changed files with 772 additions and 24 deletions.
22 changes: 11 additions & 11 deletions controllers/btpoperator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ var (
Version: btpOperatorApiVer,
Kind: btpOperatorServiceBinding,
}
instanceGvk = schema.GroupVersionKind{
InstanceGvk = schema.GroupVersionKind{
Group: btpOperatorGroup,
Version: btpOperatorApiVer,
Kind: btpOperatorServiceInstance,
Expand Down Expand Up @@ -751,7 +751,7 @@ func (r *BtpOperatorReconciler) handleDeleting(ctx context.Context, cr *v1alpha1
if err != nil {
return err
}
numberOfInstances, err := r.numberOfResources(ctx, instanceGvk)
numberOfInstances, err := r.numberOfResources(ctx, InstanceGvk)
if err != nil {
return err
}
Expand Down Expand Up @@ -799,7 +799,7 @@ func (r *BtpOperatorReconciler) handleDeprovisioning(ctx context.Context, cr *v1
if err != nil {
return err
}
numberOfInstances, err := r.numberOfResources(ctx, instanceGvk)
numberOfInstances, err := r.numberOfResources(ctx, InstanceGvk)
if err != nil {
return err
}
Expand Down Expand Up @@ -896,13 +896,13 @@ func (r *BtpOperatorReconciler) handleHardDelete(ctx context.Context, namespaces
}
}

siCrdExists, err := r.crdExists(ctx, instanceGvk)
siCrdExists, err := r.crdExists(ctx, InstanceGvk)
if err != nil {
logger.Error(err, "while checking CRD existence", "GVK", instanceGvk.String())
logger.Error(err, "while checking CRD existence", "GVK", InstanceGvk.String())
errs = append(errs, err)
}
if siCrdExists {
if err := r.hardDelete(ctx, instanceGvk, namespaces); err != nil {
if err := r.hardDelete(ctx, InstanceGvk, namespaces); err != nil {
logger.Error(err, "while deleting Service Instances")
if !errors.Is(err, context.DeadlineExceeded) {
errs = append(errs, err)
Expand Down Expand Up @@ -933,7 +933,7 @@ func (r *BtpOperatorReconciler) handleHardDelete(ctx context.Context, namespaces
}

if siCrdExists {
siResourcesLeft, err = r.resourcesExist(ctx, namespaces, instanceGvk)
siResourcesLeft, err = r.resourcesExist(ctx, namespaces, InstanceGvk)
if err != nil {
logger.Error(err, "ServiceInstance leftover resources check failed")
hardDeleteSucceededCh <- false
Expand Down Expand Up @@ -1089,9 +1089,9 @@ func (r *BtpOperatorReconciler) handleSoftDelete(ctx context.Context, namespaces
return err
}

siCrdExists, err := r.crdExists(ctx, instanceGvk)
siCrdExists, err := r.crdExists(ctx, InstanceGvk)
if err != nil {
logger.Error(err, "while checking CRD existence", "GVK", instanceGvk.String())
logger.Error(err, "while checking CRD existence", "GVK", InstanceGvk.String())
return err
}

Expand All @@ -1109,11 +1109,11 @@ func (r *BtpOperatorReconciler) handleSoftDelete(ctx context.Context, namespaces

if siCrdExists {
logger.Info("Removing finalizers in Service Instances")
if err := r.softDelete(ctx, instanceGvk); err != nil {
if err := r.softDelete(ctx, InstanceGvk); err != nil {
logger.Error(err, "while deleting Service Instances")
return err
}
if err := r.ensureResourcesDontExist(ctx, instanceGvk); err != nil {
if err := r.ensureResourcesDontExist(ctx, InstanceGvk); err != nil {
logger.Error(err, "Service Instances still exist")
return err
}
Expand Down
4 changes: 2 additions & 2 deletions controllers/btpoperator_controller_cr_rotation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ var _ = Describe("BTP Operator CR leader replacement", func() {
Eventually(func() error { return k8sClient.Create(ctx, btpOperator1) }).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(Succeed())
Eventually(updateCh).Should(Receive(matchState(v1alpha1.StateReady)))

siUnstructured := createResource(instanceGvk, kymaNamespace, instanceName)
ensureResourceExists(instanceGvk)
siUnstructured := createResource(InstanceGvk, kymaNamespace, instanceName)
ensureResourceExists(InstanceGvk)

sbUnstructured := createResource(bindingGvk, kymaNamespace, bindingName)
ensureResourceExists(bindingGvk)
Expand Down
8 changes: 4 additions & 4 deletions controllers/btpoperator_controller_deprovisioning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ var _ = Describe("BTP Operator controller - deprovisioning", func() {
})

It("Delete should fail because of existing instances and bindings", func() {
_ = createResource(instanceGvk, kymaNamespace, instanceName)
ensureResourceExists(instanceGvk)
_ = createResource(InstanceGvk, kymaNamespace, instanceName)
ensureResourceExists(InstanceGvk)

_ = createResource(bindingGvk, kymaNamespace, bindingName)
ensureResourceExists(bindingGvk)
Expand Down Expand Up @@ -78,8 +78,8 @@ var _ = Describe("BTP Operator controller - deprovisioning", func() {
btpServiceOperatorDeployment := &appsv1.Deployment{}
Expect(k8sClient.Get(ctx, client.ObjectKey{Name: DeploymentName, Namespace: kymaNamespace}, btpServiceOperatorDeployment)).Should(Succeed())

siUnstructured = createResource(instanceGvk, kymaNamespace, instanceName)
ensureResourceExists(instanceGvk)
siUnstructured = createResource(InstanceGvk, kymaNamespace, instanceName)
ensureResourceExists(InstanceGvk)

sbUnstructured = createResource(bindingGvk, kymaNamespace, bindingName)
ensureResourceExists(bindingGvk)
Expand Down
4 changes: 2 additions & 2 deletions controllers/serviceinstance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
logger.Info("SI reconcile triggered")

list := &unstructured.UnstructuredList{}
list.SetGroupVersionKind(instanceGvk)
list.SetGroupVersionKind(InstanceGvk)
err := r.List(ctx, list, client.InNamespace(corev1.NamespaceAll))
if err != nil {
return ctrl.Result{}, err
Expand Down Expand Up @@ -89,7 +89,7 @@ func (r *ServiceInstanceReconciler) SetupWithManager(mgr ctrl.Manager) error {
r.Config = mgr.GetConfig()

si := &unstructured.Unstructured{}
si.SetGroupVersionKind(instanceGvk)
si.SetGroupVersionKind(InstanceGvk)
sb := &unstructured.Unstructured{}
sb.SetGroupVersionKind(bindingGvk)

Expand Down
4 changes: 2 additions & 2 deletions controllers/serviceinstance_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ var _ = Describe("Service Instance and Bindings controller", Ordered, func() {
Eventually(updateCh).Should(Receive(matchState(v1alpha1.StateReady)))

// - create Service Instance
siUnstructured := createResource(instanceGvk, kymaNamespace, serviceInstanceName)
ensureResourceExists(instanceGvk)
siUnstructured := createResource(InstanceGvk, kymaNamespace, serviceInstanceName)
ensureResourceExists(InstanceGvk)

// - trigger BTP operator deletion
Expect(k8sClient.Delete(ctx, btpOperatorResource)).To(Succeed())
Expand Down
6 changes: 3 additions & 3 deletions controllers/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func newTimeoutK8sClient(c client.Client) *timeoutK8sClient {

func (c *timeoutK8sClient) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error {
kind := obj.GetObjectKind().GroupVersionKind().Kind
if kind == instanceGvk.Kind || kind == bindingGvk.Kind {
if kind == InstanceGvk.Kind || kind == bindingGvk.Kind {
deleteAllOfCtx, cancel := context.WithTimeout(ctx, time.Millisecond*100)
defer cancel()
return c.Client.DeleteAllOf(deleteAllOfCtx, obj, opts...)
Expand All @@ -96,7 +96,7 @@ func newErrorK8sClient(c client.Client) *errorK8sClient {

func (c *errorK8sClient) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error {
kind := obj.GetObjectKind().GroupVersionKind().Kind
if kind == instanceGvk.Kind || kind == bindingGvk.Kind {
if kind == InstanceGvk.Kind || kind == bindingGvk.Kind {
deleteAllOfCtx, cancel := context.WithTimeout(ctx, time.Millisecond*100)
defer cancel()
_ = c.Client.DeleteAllOf(deleteAllOfCtx, obj, opts...)
Expand Down Expand Up @@ -467,7 +467,7 @@ func createResource(gvk schema.GroupVersionKind, namespace string, name string)
object.SetNamespace(namespace)
object.SetName(name)
kind := object.GetObjectKind().GroupVersionKind().Kind
if kind == instanceGvk.Kind {
if kind == InstanceGvk.Kind {
populateServiceInstanceFields(object)
} else if kind == bindingGvk.Kind {
populateServiceBindingFields(object)
Expand Down
44 changes: 44 additions & 0 deletions internal/cluster-object/namespace_provider.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package clusterobject

import (
"context"
"errors"
"log/slog"

v1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

const namespaceProviderName = "NamespaceProvider"

type NamespaceProvider struct {
client.Reader
logger *slog.Logger
}

func NewNamespaceProvider(reader client.Reader, logger *slog.Logger) *NamespaceProvider {
logger = logger.With(logComponentNameKey, namespaceProviderName)

return &NamespaceProvider{
Reader: reader,
logger: logger,
}
}

func (p *NamespaceProvider) All(ctx context.Context) (*v1.NamespaceList, error) {
p.logger.Info("fetching all namespaces")

namespaces := &v1.NamespaceList{}
if err := p.Reader.List(ctx, namespaces); err != nil {
p.logger.Error("failed to fetch all namespaces", "error", err)
return nil, err
}

if len(namespaces.Items) == 0 {
err := errors.New("no namespaces found")
p.logger.Error(err.Error())
return nil, err
}

return namespaces, nil
}
69 changes: 69 additions & 0 deletions internal/cluster-object/namespace_provider_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package clusterobject

import (
"context"
"log/slog"
"os"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func TestNamespaceProvider(t *testing.T) {
// given
logger := slog.New(slog.NewTextHandler(os.Stdout, nil))

t.Run("should fetch all namespaces", func(t *testing.T) {
// given
namespaces := initNamespaces()
k8sClient := fake.NewClientBuilder().WithLists(namespaces).Build()
nsProvider := NewNamespaceProvider(k8sClient, logger)

// when
nsList, err := nsProvider.All(context.TODO())

// then
if err != nil {
t.Errorf("Error while fetching namespaces: %s", err)
}
assert.Len(t, nsList.Items, 3)
})

t.Run("should return error when no namespaces found", func(t *testing.T) {
// given
k8sClient := fake.NewClientBuilder().Build()
nsProvider := NewNamespaceProvider(k8sClient, logger)

// when
_, err := nsProvider.All(context.TODO())

// then
require.Error(t, err)
})
}

func initNamespaces() *corev1.NamespaceList {
return &corev1.NamespaceList{
Items: []corev1.Namespace{
{
ObjectMeta: metav1.ObjectMeta{
Name: "kube-system",
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "default",
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
},
}
}
17 changes: 17 additions & 0 deletions internal/cluster-object/object_provider.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package clusterobject

import (
"context"

"sigs.k8s.io/controller-runtime/pkg/client"
)

const logComponentNameKey = "component"

type ClusterScopedProvider[T client.ObjectList] interface {
All(ctx context.Context) (T, error)
}

type NamespacedProvider[T client.Object] interface {
GetByNameAndNamespace(ctx context.Context, name, namespace string) (T, error)
}
Loading

0 comments on commit 21d3779

Please sign in to comment.