Validate policy type when storing an enrich policy #48126
Conversation
martijnvg
added
>non-issue
:Data Management/Ingest Node
|
Pinging @elastic/es-core-features (:Core/Features/Ingest) |
There was a problem hiding this comment.
One question, also, is this the only thing we should be validating at PUT time? I know the PUT rquest, if security is enabled, validates that the user can use the index, but for insecure clusters it does no such validation on whether the backing index exists. Should we add this somewhere?
| @@ -53,7 +55,11 @@ public static void putPolicy(String name, EnrichPolicy policy, ClusterService cl | |||
| if (name.toLowerCase(Locale.ROOT).equals(name) == false) { | |||
| throw new IllegalArgumentException("Invalid policy name [" + name + "], must be lowercase"); | |||
| } | |||
| // TODO: add policy validation | |||
| Set<String> supportedPolicyTypes = Set.of(EnrichPolicy.SUPPORTED_POLICY_TYPES); | |||
There was a problem hiding this comment.
Should we put this validation into the constructors of the EnrichPolicy instead?
There was a problem hiding this comment.
The reason I put it here, is because it makes generating random policies easier and
down the line I think that determining what types are valid isn't going to be checking
whether the matches with an entry in EnrichPolicy.SUPPORTED_POLICY_TYPES.
So for clusters with security enabled, it checks whether there are permissions to use the source index, but not whether the source index really exists. The check whether a source index exists, happens when executing the policy. We can also check for the existence of the source index in |
I've changed my mind. I think it is helpful if the enrich policy is in a valid state from the beginning. Mainly because a policy can't be updated. I think it is okay, if we require that the source exists prior to storing the enrich policy. I will address this is a separate PR. |
No description provided.