Skip to content

Commit

Permalink
Fix the web-hook to ignore resources with a deletion timestamp set (#655
Browse files Browse the repository at this point in the history
)
  • Loading branch information
thegridman authored Jun 12, 2024
1 parent b8c03ae commit 7253283
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
26 changes: 25 additions & 1 deletion api/v1/coherence_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ func SetCommonDefaults(in CoherenceResource) {
logger := webhookLogger.WithValues("namespace", in.GetNamespace(), "name", in.GetName())
status := in.GetStatus()
spec := in.GetSpec()
dt := in.GetDeletionTimestamp()

if dt != nil {
// the deletion timestamp is set so do nothing
logger.Info("Skipping updating defaults for deleted resource", "deletionTimestamp", *dt)
return
}

if status.Phase == "" {
logger.Info("Setting defaults for new resource")

Expand Down Expand Up @@ -105,8 +113,8 @@ func SetCommonDefaults(in CoherenceResource) {
// Set the features supported by this version
in.AddAnnotation(AnnotationFeatureSuspend, "true")
} else {
logger.Info("Updating defaults for existing resource")
// this is an update
logger.Info("Updating defaults for existing resource")
}

// apply the Operator version annotation
Expand Down Expand Up @@ -134,8 +142,16 @@ var commonWebHook = CommonWebHook{}
// The optional warnings will be added to the response as warning messages.
// Return an error if the object is invalid.
func (in *Coherence) ValidateCreate() (admission.Warnings, error) {
logger := webhookLogger.WithValues("namespace", in.GetNamespace(), "name", in.GetName())
var warnings admission.Warnings

dt := in.GetDeletionTimestamp()
if dt != nil {
// the deletion timestamp is set so do nothing
logger.Info("Skipping validation for deleted resource", "deletionTimestamp", *dt)
return warnings, nil
}

webhookLogger.Info("validate create", "name", in.Name)
if err := commonWebHook.validateReplicas(in); err != nil {
return warnings, err
Expand All @@ -154,8 +170,16 @@ func (in *Coherence) ValidateCreate() (admission.Warnings, error) {
// Return an error if the object is invalid.
func (in *Coherence) ValidateUpdate(previous runtime.Object) (admission.Warnings, error) {
webhookLogger.Info("validate update", "name", in.Name)
logger := webhookLogger.WithValues("namespace", in.GetNamespace(), "name", in.GetName())
var warnings admission.Warnings

dt := in.GetDeletionTimestamp()
if dt != nil {
// the deletion timestamp is set so do nothing
logger.Info("Skipping validation for deleted resource", "deletionTimestamp", *dt)
return warnings, nil
}

if err := commonWebHook.validateReplicas(in); err != nil {
return warnings, err
}
Expand Down
21 changes: 21 additions & 0 deletions api/v1/coherence_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/ptr"
"testing"
"time"
)

func TestDefaultReplicasIsSet(t *testing.T) {
Expand Down Expand Up @@ -77,6 +78,26 @@ func TestShouldNotRemoveFinalizersAlreadyPresent(t *testing.T) {
g.Expect(finalizers).To(ContainElement(coh.CoherenceFinalizer))
}

func TestNoNotAddFinalizerToDeletedResource(t *testing.T) {
g := NewGomegaWithT(t)

dt := &metav1.Time{
Time: time.Now(),
}

c := &coh.Coherence{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
DeletionTimestamp: dt,
},
Spec: coh.CoherenceStatefulSetResourceSpec{},
}

c.Default()
finalizers := c.GetFinalizers()
g.Expect(finalizers).To(BeNil())
}

func TestDefaultReplicasIsNotOverriddenWhenAlreadySet(t *testing.T) {
g := NewGomegaWithT(t)

Expand Down

0 comments on commit 7253283

Please sign in to comment.