No license validation in webhook if Cluster is deleting #7417
Conversation
pvsune
changed the title
| @@ -684,14 +684,17 @@ func (r *Cluster) validateRedpandaResources( | |||
|
|
|||
| func (r *Cluster) validateLicense(old *Cluster) field.ErrorList { | |||
| var allErrs field.ErrorList | |||
| // Cluster has finalizers now, no validation if it is deleting | |||
| if r.GetDeletionTimestamp() != nil { | |||
There was a problem hiding this comment.
should we disable more validation when deleting? @joejulian
| if l := r.Spec.LicenseRef; l != nil { | ||
| key := &SecretKeyRef{Namespace: l.Namespace, Name: l.Name, Key: l.Key} | ||
| secret, err := key.GetSecret(context.Background(), kclient) | ||
| secret, err := l.GetSecret(context.Background(), kclient) |
There was a problem hiding this comment.
Could you change from context background to context with timeout?
There was a problem hiding this comment.
Is this a blocker? I would like to merge this ASAP to unblock @alenkacz if I pushed a change it will take around 2.5hrs for the CI to finish.
There was a problem hiding this comment.
It can be done in follow up
| @@ -633,6 +633,18 @@ func TestValidateUpdate_NoError(t *testing.T) { | |||
| assert.Error(t, err) | |||
| }) | |||
|
|
|||
| t.Run("cluster can be deleted even if licenseRef not found", func(t *testing.T) { | |||
There was a problem hiding this comment.
Am I right in thinking that when we create cluster that has valid licenseRef. Then we remove the secret with the license we would not be able to update cluster?
There was a problem hiding this comment.
Yes since it is part of the spec. If user wants to delete the license, it should also be removed in the spec - not only the referenced secret.
There was a problem hiding this comment.
My personal preference would be to flag that there is a problem with the license during updates. Any update should be possible once cluster have some license.
There was a problem hiding this comment.
The desired state, spec, wouldn't be correct then as the licenseRef in the spec is ignored once the license is loaded (e.g. the referenced secret can be deleted once it is loaded?)
There was a problem hiding this comment.
Maybe it is debatable if license kubernetes secret is gone, then this should prevent any update to the cluster custom resource.
My preference would be to log an error rather than fail it hard. Other solution would be to disable whole validation (disable webhook) in case of an incident.
There was a problem hiding this comment.
Or just delete the licenseRef as well in case of incident, the license is not enforced at Redpanda
…nse-check-deleting No license validation in webhook if Cluster is deleting
…cense-check-deleting No license validation in webhook if Cluster is deleting
We have introduced setting finalizers on the Cluster and this prohibits the cluster being deleted when the licenseRef is not found. This PR removes the validation in webhook for the licenseRef if the Cluster is in deleting state.
Backports Required
UX Changes
Release Notes
Bug Fixes