[domain-deletion]Introduce a feature flag to control domain deletion #6920
Conversation
gazi-yestemirova
requested review from
Shaddoll,
neil-xie,
davidporter-id-au,
Groxx,
shijiesheng,
jakobht,
3vilhamster,
sankari165,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
| @@ -154,6 +154,11 @@ func (wh *WorkflowHandler) DeleteDomain(ctx context.Context, deleteRequest *type | |||
| if err := wh.requestValidator.ValidateDeleteDomainRequest(ctx, deleteRequest); err != nil { | |||
| return err | |||
| } | |||
|
|
|||
| if !wh.config.EnableDomainDeletion() { | |||
| return &types.BadRequestError{Message: "Domain deletion is not enabled."} | |||
There was a problem hiding this comment.
nit: I'd change this to "Domain deletion feature is not enabled" so it is clear it is not something not enabled for the particular domain.
There was a problem hiding this comment.
At the same time what could be wrong with using this feature?
Is this allowed for a random persons?
There was a problem hiding this comment.
It is related to the previous PR and the discussion #6918 (review)
The problem is that we are introducing a new replication task requiring cross-cluster coordination.
@gazi-yestemirova Correct me if I'm wrong :)
There was a problem hiding this comment.
yes, that is correct
There was a problem hiding this comment.
Could you add this to the PR description and point to the issue with replication tasks so it will be easier to understand why we need a feature flag in this case?
|
Could you elaborate on the backward-compatibility of the feature? |
When we delete a domain, if it is a global domain, we trigger a replication task to delete the domain in a replica cluster. Sometimes, it might happen so that Cluster A has these IDL changes but Cluster B is not deployed yet (or even when the rollbacks happen). Cluster A starts generating delete tasks, which get replicated to cluster B. When cluster B doesn't recognize it and falls into the default case and the task will be discarded. Nothing will crash, but we will end up with inconsistent data across active - passive clusters. So this feature flag will be enabled when we have deployed the API changes and removed after the feature is rolled out across all the production environments. |
Does that mean you will deprecate this flag afterwards? |
Yes, it will be removed. After I roll out the domain deletion feature to all the waves |
What changed?
This PR introduces a new feature flag,
EnableDomainDeletion, to control access to the domain deletion API.This flag now gates the DeleteDomain API. If the flag is disabled, domain deletion requests will be rejected.
The flag defaults to false to prevent accidental domain deletions during rollout.
Why?
The primary goal is to make the domain deletion feature backward-compatible. We will enable the flag after the gradual rollout of the domain deletion API across all clusters.
How did you test it?
Unit tests & local testing
Potential risks
Release notes
Documentation Changes