Skip to content

Commit

Permalink
subscribe to secret change (#486)
Browse files Browse the repository at this point in the history
  • Loading branch information
kerenlahav authored Dec 23, 2024
1 parent 40fb8c4 commit cca5333
Show file tree
Hide file tree
Showing 19 changed files with 795 additions and 49 deletions.
2 changes: 2 additions & 0 deletions api/common/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package common
const (
ManagedByBTPOperatorLabel = "services.cloud.sap.com/managed-by-sap-btp-operator"
ClusterSecretLabel = "services.cloud.sap.com/cluster-secret"
InstanceSecretRefLabel = "services.cloud.sap.com/secret-ref_"
WatchSecretAnnotation = "services.cloud.sap.com/watch-secret-"

NamespaceLabel = "_namespace"
K8sNameLabel = "_k8sname"
Expand Down
11 changes: 11 additions & 0 deletions api/v1/serviceinstance_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ type ServiceInstanceSpec struct {
// +optional
ParametersFrom []ParametersFromSource `json:"parametersFrom,omitempty"`

// indicate instance will update on secrets from parametersFrom change
// +optional
WatchParametersFromChanges *bool `json:"watchParametersFromChanges,omitempty"`

// List of custom tags describing the ServiceInstance, will be copied to `ServiceBinding` secret in the key called `tags`.
// +optional
CustomTags []string `json:"customTags,omitempty"`
Expand Down Expand Up @@ -120,6 +124,9 @@ type ServiceInstanceStatus struct {

// The subaccount id of the service instance
SubaccountID string `json:"subaccountID,omitempty"`

// if true need to update instance
ForceReconcile bool `json:"forceReconcile,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down Expand Up @@ -205,6 +212,10 @@ func (si *ServiceInstance) GetShared() bool {
return si.Spec.Shared != nil && *si.Spec.Shared
}

func (si *ServiceInstance) IsSubscribedToParamSecretsChanges() bool {
return si.Spec.WatchParametersFromChanges != nil && *si.Spec.WatchParametersFromChanges
}

func (si *ServiceInstance) GetSpecHash() string {
spec := si.Spec
spec.Shared = ptr.To(false)
Expand Down
86 changes: 86 additions & 0 deletions api/v1/serviceinstance_types_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package v1

import (
"crypto/md5"
"encoding/hex"
"encoding/json"

"github.com/SAP/sap-btp-service-operator/api/common"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/utils/ptr"
)

var _ = Describe("Service Instance Type Test", func() {
Expand Down Expand Up @@ -123,4 +128,85 @@ var _ = Describe("Service Instance Type Test", func() {
instance.SetAnnotations(annotation)
Expect(instance.GetAnnotations()).To(Equal(annotation))
})

It("should update WatchParametersFromChanges", func() {
instance.Spec.WatchParametersFromChanges = &[]bool{true}[0]
Expect(instance.IsSubscribedToParamSecretsChanges()).To(BeTrue())
})

It("should return correct spec hash", func() {
// Calculate expected hash
spec := instance.Spec
spec.Shared = ptr.To(false)
specBytes, _ := json.Marshal(spec)
hash := md5.Sum(specBytes)
expectedHash := hex.EncodeToString(hash[:])

// Get actual hash
actualHash := instance.GetSpecHash()

// Compare hashes
Expect(actualHash).To(Equal(expectedHash))
})
It("should update spec hash when spec changes", func() {
// Calculate initial hash
initialHash := instance.GetSpecHash()

// Modify the spec
instance.Spec.ServicePlanName = "new-plan"

// Calculate new hash
newHash := instance.GetSpecHash()

// Ensure the hash has changed
Expect(initialHash).NotTo(Equal(newHash))
})
It("should update spec hash when parametersFrom changes", func() {
// Calculate initial hash
initialHash := instance.GetSpecHash()

// Modify the parametersFrom field
instance.Spec.ParametersFrom = []ParametersFromSource{
{
SecretKeyRef: &SecretKeyReference{
Name: "new-param-secret",
Key: "new-secret-parameter",
},
},
}

// Calculate new hash
newHash := instance.GetSpecHash()

// Ensure the hash has changed
Expect(initialHash).NotTo(Equal(newHash))
})
It("should update spec hash when parametersFrom changes with initial object", func() {
// Initialize ParametersFrom with an object
instance.Spec.ParametersFrom = []ParametersFromSource{
{
SecretKeyRef: &SecretKeyReference{
Name: "initial-param-secret",
Key: "initial-secret-parameter",
},
},
}

// Calculate initial hash
initialHash := instance.GetSpecHash()

// Modify the parametersFrom field
instance.Spec.ParametersFrom = append(instance.Spec.ParametersFrom, ParametersFromSource{
SecretKeyRef: &SecretKeyReference{
Name: "new-param-secret",
Key: "new-secret-parameter",
},
})

// Calculate new hash
newHash := instance.GetSpecHash()

// Ensure the hash has changed
Expect(initialHash).NotTo(Equal(newHash))
})
})
1 change: 1 addition & 0 deletions api/v1/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func getInstance() *ServiceInstance {
},
},
},
WatchParametersFromChanges: &[]bool{true}[0],
UserInfo: &v1.UserInfo{
Username: "test-user",
Groups: []string{"test-group"},
Expand Down
5 changes: 5 additions & 0 deletions api/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions config/crd/bases/services.cloud.sap.com_serviceinstances.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ spec:
shared:
description: Indicates the desired shared state
type: boolean
watchParametersFromChanges:
description: indicate instance will update on secrets from parametersFrom
change
type: boolean
userInfo:
description: |-
UserInfo contains information about the user that last modified this
Expand Down Expand Up @@ -248,6 +252,9 @@ spec:
- type
type: object
type: array
forceReconcile:
description: if true need to update instance
type: boolean
hashedSpec:
description: HashedSpec is the hashed spec without the shared property
type: string
Expand Down
126 changes: 126 additions & 0 deletions controllers/secret_controller.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
package controllers

import (
"context"
"fmt"
"reflect"

"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"

"github.com/SAP/sap-btp-service-operator/api/common"
v1 "github.com/SAP/sap-btp-service-operator/api/v1"
"github.com/SAP/sap-btp-service-operator/internal/utils"
"github.com/go-logr/logr"
"github.com/google/uuid"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

type SecretReconciler struct {
client.Client
Scheme *runtime.Scheme
Log logr.Logger
}

// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=core,resources=events,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;create;update

func (r *SecretReconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
log := r.Log.WithValues("secret", req.NamespacedName).WithValues("correlation_id", uuid.New().String())
ctx = context.WithValue(ctx, utils.LogKey{}, log)
log.Info(fmt.Sprintf("reconciling params secret %s", req.NamespacedName))
// Fetch the Secret
secret := &corev1.Secret{}
if err := r.Get(ctx, req.NamespacedName, secret); err != nil {
if !apierrors.IsNotFound(err) {
log.Error(err, "unable to fetch Secret")
}
// we'll ignore not-found errors, since they can't be fixed by an immediate
// requeue (we'll need to wait for a new notification), and we can get them
// on deleted requests.
return ctrl.Result{}, client.IgnoreNotFound(err)
}

instances := &v1.ServiceInstanceList{}
labelSelector := client.MatchingLabels{utils.GetLabelKeyForInstanceSecret(secret.Name): secret.Name}
if err := r.Client.List(ctx, instances, client.InNamespace(secret.Namespace), labelSelector); err != nil {
log.Error(err, "failed to list service instances")
return ctrl.Result{}, err
}

for _, instance := range instances.Items {
log.Info(fmt.Sprintf("waking up referencing instance %s", instance.Name))
instance.Status.ForceReconcile = true
err := utils.UpdateStatus(ctx, r.Client, &instance)
if err != nil {
return reconcile.Result{}, err
}
}

if utils.IsMarkedForDeletion(secret.ObjectMeta) {
log.Info("secret is marked for deletion, removing finalizer")
return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, secret, common.FinalizerName)
}

log.Info("finished reconciling params secret")
return reconcile.Result{}, nil
}

// SetupWithManager sets up the controller with the Manager.
func (r *SecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
predicates := predicate.Funcs{
UpdateFunc: func(e event.UpdateEvent) bool {
return (utils.IsSecretWatched(e.ObjectNew.GetAnnotations()) && isSecretDataChanged(e)) || isSecretInDelete(e)
},
CreateFunc: func(e event.CreateEvent) bool {
return utils.IsSecretWatched(e.Object.GetAnnotations())
},
DeleteFunc: func(e event.DeleteEvent) bool {
return utils.IsSecretWatched(e.Object.GetAnnotations())
},
GenericFunc: func(e event.GenericEvent) bool {
return utils.IsSecretWatched(e.Object.GetAnnotations())
},
}

return ctrl.NewControllerManagedBy(mgr).
For(&corev1.Secret{}).
WithEventFilter(predicates).
WithOptions(controller.Options{MaxConcurrentReconciles: 1}).
Complete(r)
}

func isSecretDataChanged(e event.UpdateEvent) bool {
// Type assert to *v1.Secret
oldSecret, okOld := e.ObjectOld.(*corev1.Secret)
newSecret, okNew := e.ObjectNew.(*corev1.Secret)
if !okOld || !okNew {
// If the objects are not Secrets, skip the event
return false
}

// Compare the Data field (byte slices)
return !reflect.DeepEqual(oldSecret.Data, newSecret.Data) || !reflect.DeepEqual(oldSecret.StringData, newSecret.StringData)
}

func isSecretInDelete(e event.UpdateEvent) bool {
// Type assert to *v1.Secret

newSecret, okNew := e.ObjectNew.(*corev1.Secret)
if !okNew {
// If the objects are not Secrets, skip the event
return false
}

// Compare the Data field (byte slices)
return !newSecret.GetDeletionTimestamp().IsZero() && controllerutil.ContainsFinalizer(newSecret, common.FinalizerName)
}
2 changes: 1 addition & 1 deletion controllers/servicebinding_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s
log := utils.GetLogger(ctx)
log.Info("Creating smBinding in SM")
serviceBinding.Status.InstanceID = serviceInstance.Status.InstanceID
_, bindingParameters, err := utils.BuildSMRequestParameters(serviceBinding.Namespace, serviceBinding.Spec.ParametersFrom, serviceBinding.Spec.Parameters)
bindingParameters, _, err := utils.BuildSMRequestParameters(serviceBinding.Namespace, serviceBinding.Spec.Parameters, serviceBinding.Spec.ParametersFrom)
if err != nil {
log.Error(err, "failed to parse smBinding parameters")
return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.CREATE, err, serviceBinding)
Expand Down
10 changes: 7 additions & 3 deletions controllers/servicebinding_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ import (
"net/http"
"strings"

"github.com/SAP/sap-btp-service-operator/api/common"
"github.com/SAP/sap-btp-service-operator/internal/utils"
"github.com/lithammer/dedent"
authv1 "k8s.io/api/authentication/v1"

"github.com/SAP/sap-btp-service-operator/api/common"
"github.com/SAP/sap-btp-service-operator/internal/utils"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"

Expand Down Expand Up @@ -582,7 +583,10 @@ var _ = Describe("ServiceBinding controller", func() {
createdBinding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "", false)
Expect(err).ToNot(HaveOccurred())
waitForResourceCondition(ctx, createdBinding, common.ConditionSucceeded, metav1.ConditionFalse, common.Blocked, "")
Expect(utils.RemoveFinalizer(ctx, k8sClient, createdInstance, "fake/finalizer")).To(Succeed())
Eventually(func() bool {
err := k8sClient.Get(ctx, getResourceNamespacedName(createdInstance), createdInstance)
return err == nil && utils.RemoveFinalizer(ctx, k8sClient, createdInstance, "fake/finalizer") == nil
}, timeout, interval).Should(BeTrue())
})
})

Expand Down
Loading

0 comments on commit cca5333

Please sign in to comment.