Revert "[Transform] Report validation failure if there are no aggregations in the test search query"#95562
Merged
przemekwitek merged 1 commit intomainfrom Apr 26, 2023
Merged
Conversation
Collaborator
|
Pinging @elastic/ml-core (Team:ML) |
valeriy42
added a commit
that referenced
this pull request
Feb 12, 2026
…er lacks remote index permissions (#142403) When a transform is configured with a remote (cross-cluster) source index and the user lacks permissions to access it, the _preview API correctly fails -- but PUT _transform and _start silently succeed, allowing unauthorized transforms to be created and started. The root cause is that validateQuery in AbstractCompositeAggFunction only checks the response status code, which is OK even when IndicesOptions.LENIENT_EXPAND_OPEN causes unauthorized indices to be silently ignored. The search returns null aggregations in this case, but unlike preview(), validateQuery() never checks for that condition. This PR introduces a SourceAccessDiagnostics class that inspects the SearchResponse for security-related failures at both the CCS cluster level (SKIPPED/FAILED clusters with ElasticsearchSecurityException) and the shard level (FORBIDDEN/UNAUTHORIZED status). A null-aggregation check is added to validateQuery(), but -- critically -- it only rejects the request when a security failure is positively identified. When no security failure is found, validation passes through silently. This distinction avoids the regression that caused PR #95318 to be reverted in #95562: that earlier change unconditionally failed on null aggregations, which broke integrations (such as Elastic Defend) that create and start transforms with wildcard source patterns before any matching indices exist. Since defer_validation only defers from PUT to _start, there was no way for those integrations to bypass the check. Our approach preserves backward compatibility for the empty-indices case while catching the unauthorized-remote-index case. The preview() method also delegates to the same diagnostics class, so all three APIs now produce consistent, actionable error messages when a security failure is detected, falling back to the original generic message otherwise. The multi-cluster YAML integration tests are updated to verify that both PUT _transform and _start now reject unauthorized remote transforms. A new test case creates a transform with defer_validation: true and confirms that _start catches the permission issue. Unit tests for SourceAccessDiagnostics cover cluster-level SKIPPED/FAILED scenarios, shard-level security exceptions, FORBIDDEN/UNAUTHORIZED status codes, and the fallback to the generic message for non-security failures. Fixes #95367
valeriy42
added a commit
to valeriy42/elasticsearch
that referenced
this pull request
Feb 12, 2026
…er lacks remote index permissions (elastic#142403) When a transform is configured with a remote (cross-cluster) source index and the user lacks permissions to access it, the _preview API correctly fails -- but PUT _transform and _start silently succeed, allowing unauthorized transforms to be created and started. The root cause is that validateQuery in AbstractCompositeAggFunction only checks the response status code, which is OK even when IndicesOptions.LENIENT_EXPAND_OPEN causes unauthorized indices to be silently ignored. The search returns null aggregations in this case, but unlike preview(), validateQuery() never checks for that condition. This PR introduces a SourceAccessDiagnostics class that inspects the SearchResponse for security-related failures at both the CCS cluster level (SKIPPED/FAILED clusters with ElasticsearchSecurityException) and the shard level (FORBIDDEN/UNAUTHORIZED status). A null-aggregation check is added to validateQuery(), but -- critically -- it only rejects the request when a security failure is positively identified. When no security failure is found, validation passes through silently. This distinction avoids the regression that caused PR elastic#95318 to be reverted in elastic#95562: that earlier change unconditionally failed on null aggregations, which broke integrations (such as Elastic Defend) that create and start transforms with wildcard source patterns before any matching indices exist. Since defer_validation only defers from PUT to _start, there was no way for those integrations to bypass the check. Our approach preserves backward compatibility for the empty-indices case while catching the unauthorized-remote-index case. The preview() method also delegates to the same diagnostics class, so all three APIs now produce consistent, actionable error messages when a security failure is detected, falling back to the original generic message otherwise. The multi-cluster YAML integration tests are updated to verify that both PUT _transform and _start now reject unauthorized remote transforms. A new test case creates a transform with defer_validation: true and confirms that _start catches the permission issue. Unit tests for SourceAccessDiagnostics cover cluster-level SKIPPED/FAILED scenarios, shard-level security exceptions, FORBIDDEN/UNAUTHORIZED status codes, and the fallback to the generic message for non-security failures. Fixes elastic#95367
valeriy42
added a commit
to valeriy42/elasticsearch
that referenced
this pull request
Feb 13, 2026
…er lacks remote index permissions (elastic#142403) When a transform is configured with a remote (cross-cluster) source index and the user lacks permissions to access it, the _preview API correctly fails -- but PUT _transform and _start silently succeed, allowing unauthorized transforms to be created and started. The root cause is that validateQuery in AbstractCompositeAggFunction only checks the response status code, which is OK even when IndicesOptions.LENIENT_EXPAND_OPEN causes unauthorized indices to be silently ignored. The search returns null aggregations in this case, but unlike preview(), validateQuery() never checks for that condition. This PR introduces a SourceAccessDiagnostics class that inspects the SearchResponse for security-related failures at both the CCS cluster level (SKIPPED/FAILED clusters with ElasticsearchSecurityException) and the shard level (FORBIDDEN/UNAUTHORIZED status). A null-aggregation check is added to validateQuery(), but -- critically -- it only rejects the request when a security failure is positively identified. When no security failure is found, validation passes through silently. This distinction avoids the regression that caused PR elastic#95318 to be reverted in elastic#95562: that earlier change unconditionally failed on null aggregations, which broke integrations (such as Elastic Defend) that create and start transforms with wildcard source patterns before any matching indices exist. Since defer_validation only defers from PUT to _start, there was no way for those integrations to bypass the check. Our approach preserves backward compatibility for the empty-indices case while catching the unauthorized-remote-index case. The preview() method also delegates to the same diagnostics class, so all three APIs now produce consistent, actionable error messages when a security failure is detected, falling back to the original generic message otherwise. The multi-cluster YAML integration tests are updated to verify that both PUT _transform and _start now reject unauthorized remote transforms. A new test case creates a transform with defer_validation: true and confirms that _start catches the permission issue. Unit tests for SourceAccessDiagnostics cover cluster-level SKIPPED/FAILED scenarios, shard-level security exceptions, FORBIDDEN/UNAUTHORIZED status codes, and the fallback to the generic message for non-security failures. Fixes elastic#95367 (cherry picked from commit 0e44984) # Conflicts: # x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/common/AbstractCompositeAggFunction.java
valeriy42
added a commit
to valeriy42/elasticsearch
that referenced
this pull request
Feb 13, 2026
…er lacks remote index permissions (elastic#142403) When a transform is configured with a remote (cross-cluster) source index and the user lacks permissions to access it, the _preview API correctly fails -- but PUT _transform and _start silently succeed, allowing unauthorized transforms to be created and started. The root cause is that validateQuery in AbstractCompositeAggFunction only checks the response status code, which is OK even when IndicesOptions.LENIENT_EXPAND_OPEN causes unauthorized indices to be silently ignored. The search returns null aggregations in this case, but unlike preview(), validateQuery() never checks for that condition. This PR introduces a SourceAccessDiagnostics class that inspects the SearchResponse for security-related failures at both the CCS cluster level (SKIPPED/FAILED clusters with ElasticsearchSecurityException) and the shard level (FORBIDDEN/UNAUTHORIZED status). A null-aggregation check is added to validateQuery(), but -- critically -- it only rejects the request when a security failure is positively identified. When no security failure is found, validation passes through silently. This distinction avoids the regression that caused PR elastic#95318 to be reverted in elastic#95562: that earlier change unconditionally failed on null aggregations, which broke integrations (such as Elastic Defend) that create and start transforms with wildcard source patterns before any matching indices exist. Since defer_validation only defers from PUT to _start, there was no way for those integrations to bypass the check. Our approach preserves backward compatibility for the empty-indices case while catching the unauthorized-remote-index case. The preview() method also delegates to the same diagnostics class, so all three APIs now produce consistent, actionable error messages when a security failure is detected, falling back to the original generic message otherwise. The multi-cluster YAML integration tests are updated to verify that both PUT _transform and _start now reject unauthorized remote transforms. A new test case creates a transform with defer_validation: true and confirms that _start catches the permission issue. Unit tests for SourceAccessDiagnostics cover cluster-level SKIPPED/FAILED scenarios, shard-level security exceptions, FORBIDDEN/UNAUTHORIZED status codes, and the fallback to the generic message for non-security failures. Fixes elastic#95367 (cherry picked from commit 0e44984) # Conflicts: # x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/common/AbstractCompositeAggFunction.java
elasticsearchmachine
pushed a commit
that referenced
this pull request
Feb 13, 2026
…hen user lacks remote index permissions (#142403) (#142454) * [Transform] Fix transform validation to reject PUT and _start when user lacks remote index permissions (#142403) When a transform is configured with a remote (cross-cluster) source index and the user lacks permissions to access it, the _preview API correctly fails -- but PUT _transform and _start silently succeed, allowing unauthorized transforms to be created and started. The root cause is that validateQuery in AbstractCompositeAggFunction only checks the response status code, which is OK even when IndicesOptions.LENIENT_EXPAND_OPEN causes unauthorized indices to be silently ignored. The search returns null aggregations in this case, but unlike preview(), validateQuery() never checks for that condition. This PR introduces a SourceAccessDiagnostics class that inspects the SearchResponse for security-related failures at both the CCS cluster level (SKIPPED/FAILED clusters with ElasticsearchSecurityException) and the shard level (FORBIDDEN/UNAUTHORIZED status). A null-aggregation check is added to validateQuery(), but -- critically -- it only rejects the request when a security failure is positively identified. When no security failure is found, validation passes through silently. This distinction avoids the regression that caused PR #95318 to be reverted in #95562: that earlier change unconditionally failed on null aggregations, which broke integrations (such as Elastic Defend) that create and start transforms with wildcard source patterns before any matching indices exist. Since defer_validation only defers from PUT to _start, there was no way for those integrations to bypass the check. Our approach preserves backward compatibility for the empty-indices case while catching the unauthorized-remote-index case. The preview() method also delegates to the same diagnostics class, so all three APIs now produce consistent, actionable error messages when a security failure is detected, falling back to the original generic message otherwise. The multi-cluster YAML integration tests are updated to verify that both PUT _transform and _start now reject unauthorized remote transforms. A new test case creates a transform with defer_validation: true and confirms that _start catches the permission issue. Unit tests for SourceAccessDiagnostics cover cluster-level SKIPPED/FAILED scenarios, shard-level security exceptions, FORBIDDEN/UNAUTHORIZED status codes, and the fallback to the generic message for non-security failures. Fixes #95367 (cherry picked from commit 0e44984) # Conflicts: # x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/common/AbstractCompositeAggFunction.java * Fix SearchResponse.Cluster constructor arity in SourceAccessDiagnosticsTests * checkstyle
elasticsearchmachine
pushed a commit
that referenced
this pull request
Feb 13, 2026
…hen user lacks remote index permissions (#142403) (#142430) * [Transform] Fix transform validation to reject PUT and _start when user lacks remote index permissions (#142403) When a transform is configured with a remote (cross-cluster) source index and the user lacks permissions to access it, the _preview API correctly fails -- but PUT _transform and _start silently succeed, allowing unauthorized transforms to be created and started. The root cause is that validateQuery in AbstractCompositeAggFunction only checks the response status code, which is OK even when IndicesOptions.LENIENT_EXPAND_OPEN causes unauthorized indices to be silently ignored. The search returns null aggregations in this case, but unlike preview(), validateQuery() never checks for that condition. This PR introduces a SourceAccessDiagnostics class that inspects the SearchResponse for security-related failures at both the CCS cluster level (SKIPPED/FAILED clusters with ElasticsearchSecurityException) and the shard level (FORBIDDEN/UNAUTHORIZED status). A null-aggregation check is added to validateQuery(), but -- critically -- it only rejects the request when a security failure is positively identified. When no security failure is found, validation passes through silently. This distinction avoids the regression that caused PR #95318 to be reverted in #95562: that earlier change unconditionally failed on null aggregations, which broke integrations (such as Elastic Defend) that create and start transforms with wildcard source patterns before any matching indices exist. Since defer_validation only defers from PUT to _start, there was no way for those integrations to bypass the check. Our approach preserves backward compatibility for the empty-indices case while catching the unauthorized-remote-index case. The preview() method also delegates to the same diagnostics class, so all three APIs now produce consistent, actionable error messages when a security failure is detected, falling back to the original generic message otherwise. The multi-cluster YAML integration tests are updated to verify that both PUT _transform and _start now reject unauthorized remote transforms. A new test case creates a transform with defer_validation: true and confirms that _start catches the permission issue. Unit tests for SourceAccessDiagnostics cover cluster-level SKIPPED/FAILED scenarios, shard-level security exceptions, FORBIDDEN/UNAUTHORIZED status codes, and the fallback to the generic message for non-security failures. Fixes #95367 * Fix SearchResponse.Cluster constructor arity in SourceAccessDiagnosticsTests * checkstyle
sidosera
pushed a commit
to sidosera/elasticsearch
that referenced
this pull request
Feb 13, 2026
…er lacks remote index permissions (elastic#142403) When a transform is configured with a remote (cross-cluster) source index and the user lacks permissions to access it, the _preview API correctly fails -- but PUT _transform and _start silently succeed, allowing unauthorized transforms to be created and started. The root cause is that validateQuery in AbstractCompositeAggFunction only checks the response status code, which is OK even when IndicesOptions.LENIENT_EXPAND_OPEN causes unauthorized indices to be silently ignored. The search returns null aggregations in this case, but unlike preview(), validateQuery() never checks for that condition. This PR introduces a SourceAccessDiagnostics class that inspects the SearchResponse for security-related failures at both the CCS cluster level (SKIPPED/FAILED clusters with ElasticsearchSecurityException) and the shard level (FORBIDDEN/UNAUTHORIZED status). A null-aggregation check is added to validateQuery(), but -- critically -- it only rejects the request when a security failure is positively identified. When no security failure is found, validation passes through silently. This distinction avoids the regression that caused PR elastic#95318 to be reverted in elastic#95562: that earlier change unconditionally failed on null aggregations, which broke integrations (such as Elastic Defend) that create and start transforms with wildcard source patterns before any matching indices exist. Since defer_validation only defers from PUT to _start, there was no way for those integrations to bypass the check. Our approach preserves backward compatibility for the empty-indices case while catching the unauthorized-remote-index case. The preview() method also delegates to the same diagnostics class, so all three APIs now produce consistent, actionable error messages when a security failure is detected, falling back to the original generic message otherwise. The multi-cluster YAML integration tests are updated to verify that both PUT _transform and _start now reject unauthorized remote transforms. A new test case creates a transform with defer_validation: true and confirms that _start catches the permission issue. Unit tests for SourceAccessDiagnostics cover cluster-level SKIPPED/FAILED scenarios, shard-level security exceptions, FORBIDDEN/UNAUTHORIZED status codes, and the fallback to the generic message for non-security failures. Fixes elastic#95367
elasticsearchmachine
pushed a commit
that referenced
this pull request
Feb 23, 2026
…when user lacks remote index permissions (#142403) (#142455) * [Transform] Fix transform validation to reject PUT and _start when user lacks remote index permissions (#142403) When a transform is configured with a remote (cross-cluster) source index and the user lacks permissions to access it, the _preview API correctly fails -- but PUT _transform and _start silently succeed, allowing unauthorized transforms to be created and started. The root cause is that validateQuery in AbstractCompositeAggFunction only checks the response status code, which is OK even when IndicesOptions.LENIENT_EXPAND_OPEN causes unauthorized indices to be silently ignored. The search returns null aggregations in this case, but unlike preview(), validateQuery() never checks for that condition. This PR introduces a SourceAccessDiagnostics class that inspects the SearchResponse for security-related failures at both the CCS cluster level (SKIPPED/FAILED clusters with ElasticsearchSecurityException) and the shard level (FORBIDDEN/UNAUTHORIZED status). A null-aggregation check is added to validateQuery(), but -- critically -- it only rejects the request when a security failure is positively identified. When no security failure is found, validation passes through silently. This distinction avoids the regression that caused PR #95318 to be reverted in #95562: that earlier change unconditionally failed on null aggregations, which broke integrations (such as Elastic Defend) that create and start transforms with wildcard source patterns before any matching indices exist. Since defer_validation only defers from PUT to _start, there was no way for those integrations to bypass the check. Our approach preserves backward compatibility for the empty-indices case while catching the unauthorized-remote-index case. The preview() method also delegates to the same diagnostics class, so all three APIs now produce consistent, actionable error messages when a security failure is detected, falling back to the original generic message otherwise. The multi-cluster YAML integration tests are updated to verify that both PUT _transform and _start now reject unauthorized remote transforms. A new test case creates a transform with defer_validation: true and confirms that _start catches the permission issue. Unit tests for SourceAccessDiagnostics cover cluster-level SKIPPED/FAILED scenarios, shard-level security exceptions, FORBIDDEN/UNAUTHORIZED status codes, and the fallback to the generic message for non-security failures. Fixes #95367 (cherry picked from commit 0e44984) # Conflicts: # x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/common/AbstractCompositeAggFunction.java * fix compilation error * Add diagnostics for remote CCS clusters with zero shards Enhance the SourceAccessDiagnostics class to identify remote CCS clusters that return zero shards due to permission issues. This update includes a new method to check for such scenarios and updates the documentation accordingly. Additionally, new unit tests are added to verify the correct behavior when accessing remote clusters with insufficient permissions, ensuring that appropriate error messages are returned. This change improves the clarity of diagnostics related to security exceptions in cross-cluster searches. * fix unit test specifics for 8.19 * Update transform configuration in multi-cluster test to include defer_validation and modify description
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#95318 turned out to be a braking change for some integrations.
Reverting it for now.
Reverts #95318
Relates #95367