Revert "[7.17] Backport DLS changes (#108330)" (#109349)

This reverts commit 4e08df5 (#108330)
This commit also fixes #109273 in 7.17.

Author: jakelandis

docs/reference/migration/migrate_7_17.asciidoc

@@ -80,23 +80,6 @@ To avoid deprecation warnings, remove any SSL truststores that do not
 contain any trusted entries.
 ====
 
-[[deprecation_for_dls_settings]]
-.Deprecation for DLS settings
-[%collapsible]
-====
-*Details* +
-Two settings available in the latest versions of 7.17 are not available in the next major version.
-Newer versions of 7.17 default to stricter Document Level Security (DLS) rules and the follow
-settings can be used to disable those stricter DLS rules:
-`xpack.security.dls.force_terms_aggs_to_exclude_deleted_docs.enabled` and
-`xpack.security.dls.error_when_validate_query_with_rewrite.enabled`.
-Newer versions, of next major version of {es}, also default to the stricter DLS rules but don't allow
-usage of the less strict rules.
-
-*Impact* +
-To avoid deprecation warnings, remove these settings from elasticsearch.yml.
-====
-
 [discrete]
 [[deprecations_717_mapping]]
 ==== Mapping deprecations

docs/reference/release-notes/7.17.22.asciidoc

@@ -5,36 +5,6 @@ coming[7.17.22]
 
 Also see <<breaking-changes-7.17,Breaking changes in 7.17>>.
 
-[[breaking-7.17.22]]
-[float]
-=== Breaking changes
-
-[discrete]
-[[breaking_7_17_22_dls_changes]]
-==== Stricter Document Level Security (DLS)
-
-[[stricter_dls_7_17_22]]
-.Document Level Security (DLS) applies stricter checks for the validate query API and for terms aggregations when min_doc_count is set to 0.
-
-[%collapsible]
-====
-*Details* +
-When Document Level Security (DLS) is applied to terms aggregations and min_doc_count is set to 0, stricter security rules apply.
-When Document Level Security (DLS) is applied to the validate query API with the rewrite parameter, stricter security rules apply.
-
-*Impact* +
-If needed, test workflows with DLS enabled to ensure that the stricter security rules do not impact your application.
-
-*Remediation* +
-Set min_doc_count to a value greater than 0 in terms aggregations or use an account not constrained by DLS for the validate query API calls.
-
-Set `xpack.security.dls.force_terms_aggs_to_exclude_deleted_docs.enabled` to `false` in the Elasticsearch configuration
-to revert to the previous behavior.
-
-Set `xpack.security.dls.error_when_validate_query_with_rewrite.enabled` to `false` in the Elasticsearch configuration
-to revert to the previous behavior.
-====
-
 [[bug-7.17.22]]
 [float]
 === Bug fixes

server/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java

@@ -41,7 +41,6 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Queue;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -331,48 +330,6 @@ public boolean mustVisitAllDocs() {
             return false;
         }
 
-        /**
-         * Return true if any of the builders is a terms aggregation with min_doc_count=0
-         */
-        public boolean hasZeroMinDocTermsAggregation() {
-            final Queue<AggregationBuilder> queue = new LinkedList<>(aggregationBuilders);
-            while (queue.isEmpty() == false) {
-                final AggregationBuilder current = queue.poll();
-                if (current == null) {
-                    continue;
-                }
-                if (current instanceof TermsAggregationBuilder) {
-                    TermsAggregationBuilder termsBuilder = (TermsAggregationBuilder) current;
-                    if (termsBuilder.minDocCount() == 0) {
-                        return true;
-                    }
-                }
-                queue.addAll(current.getSubAggregations());
-            }
-            return false;
-        }
-
-        /**
-         * Force all min_doc_count=0 terms aggregations to exclude deleted docs.
-         */
-        public void forceTermsAggsToExcludeDeletedDocs() {
-            assert hasZeroMinDocTermsAggregation();
-            final Queue<AggregationBuilder> queue = new LinkedList<>(aggregationBuilders);
-            while (queue.isEmpty() == false) {
-                final AggregationBuilder current = queue.poll();
-                if (current == null) {
-                    continue;
-                }
-                if (current instanceof TermsAggregationBuilder) {
-                    TermsAggregationBuilder termsBuilder = (TermsAggregationBuilder) current;
-                    if (termsBuilder.minDocCount() == 0) {
-                        termsBuilder.excludeDeletedDocs(true);
-                    }
-                }
-                queue.addAll(current.getSubAggregations());
-            }
-        }
-
         public Builder addAggregator(AggregationBuilder factory) {
             if (names.add(factory.name) == false) {
                 throw new IllegalArgumentException("Two sibling aggregations cannot have the same name: [" + factory.name + "]");

server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java

@@ -9,19 +9,15 @@
 package org.elasticsearch.search.aggregations.bucket.terms;
 
 import org.apache.lucene.index.DocValues;
-import org.apache.lucene.index.LeafReader;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.SortedDocValues;
 import org.apache.lucene.index.SortedSetDocValues;
 import org.apache.lucene.util.ArrayUtil;
-import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.PriorityQueue;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.LongArray;
 import org.elasticsearch.common.util.LongHash;
-import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.Releasable;
 import org.elasticsearch.core.Releasables;
 import org.elasticsearch.search.DocValueFormat;
@@ -86,8 +82,7 @@ public GlobalOrdinalsStringTermsAggregator(
         SubAggCollectionMode collectionMode,
         boolean showTermDocCountError,
         CardinalityUpperBound cardinality,
-        Map<String, Object> metadata,
-        boolean excludeDeletedDocs
+        Map<String, Object> metadata
     ) throws IOException {
         super(name, factories, context, parent, order, format, bucketCountThresholds, collectionMode, showTermDocCountError, metadata);
         this.resultStrategy = resultStrategy.apply(this); // ResultStrategy needs a reference to the Aggregator to do its job.
@@ -96,13 +91,13 @@ public GlobalOrdinalsStringTermsAggregator(
         this.lookupGlobalOrd = values::lookupOrd;
         this.acceptedGlobalOrdinals = acceptedOrds;
         if (remapGlobalOrds) {
-            this.collectionStrategy = new RemapGlobalOrds(cardinality, excludeDeletedDocs);
+            this.collectionStrategy = new RemapGlobalOrds(cardinality);
         } else {
             this.collectionStrategy = cardinality.map(estimate -> {
                 if (estimate > 1) {
                     throw new AggregationExecutionException("Dense ords don't know how to collect from many buckets");
                 }
-                return new DenseGlobalOrds(excludeDeletedDocs);
+                return new DenseGlobalOrds();
             });
         }
     }
@@ -279,8 +274,7 @@ static class LowCardinality extends GlobalOrdinalsStringTermsAggregator {
             boolean remapGlobalOrds,
             SubAggCollectionMode collectionMode,
             boolean showTermDocCountError,
-            Map<String, Object> metadata,
-            boolean excludeDeletedDocs
+            Map<String, Object> metadata
         ) throws IOException {
             super(
                 name,
@@ -298,8 +292,7 @@ static class LowCardinality extends GlobalOrdinalsStringTermsAggregator {
                 collectionMode,
                 showTermDocCountError,
                 CardinalityUpperBound.ONE,
-                metadata,
-                excludeDeletedDocs
+                metadata
             );
             assert factories == null || factories.countAggregators() == 0;
             this.segmentDocCounts = context.bigArrays().newLongArray(1, true);
@@ -448,13 +441,6 @@ interface BucketInfoConsumer {
      * bucket ordinal.
      */
     class DenseGlobalOrds extends CollectionStrategy {
-
-        private final boolean excludeDeletedDocs;
-
-        DenseGlobalOrds(boolean excludeDeletedDocs) {
-            this.excludeDeletedDocs = excludeDeletedDocs;
-        }
-
         @Override
         String describe() {
             return "dense";
@@ -485,14 +471,6 @@ long globalOrdToBucketOrd(long owningBucketOrd, long globalOrd) {
         @Override
         void forEach(long owningBucketOrd, BucketInfoConsumer consumer) throws IOException {
             assert owningBucketOrd == 0;
-            if (excludeDeletedDocs) {
-                forEachExcludeDeletedDocs(consumer);
-            } else {
-                forEachAllowDeletedDocs(consumer);
-            }
-        }
-
-        private void forEachAllowDeletedDocs(BucketInfoConsumer consumer) throws IOException {
             for (long globalOrd = 0; globalOrd < valueCount; globalOrd++) {
                 if (false == acceptedGlobalOrdinals.test(globalOrd)) {
                     continue;
@@ -504,39 +482,6 @@ private void forEachAllowDeletedDocs(BucketInfoConsumer consumer) throws IOExcep
             }
         }
 
-        /**
-         * Excludes deleted docs in the results by cross-checking with liveDocs.
-         */
-        private void forEachExcludeDeletedDocs(BucketInfoConsumer consumer) throws IOException {
-            try (LongHash accepted = new LongHash(20, new BigArrays(null, null, ""))) {
-                for (LeafReaderContext ctx : searcher().getTopReaderContext().leaves()) {
-                    LeafReader reader = ctx.reader();
-                    Bits liveDocs = reader.getLiveDocs();
-                    SortedSetDocValues globalOrds = null;
-                    for (int docId = 0; docId < reader.maxDoc(); ++docId) {
-                        if (liveDocs == null || liveDocs.get(docId)) {  // document is not deleted
-                            globalOrds = globalOrds == null ? valuesSource.globalOrdinalsValues(ctx) : globalOrds;
-                            if (globalOrds.advanceExact(docId)) {
-                                for (long globalOrd = globalOrds.nextOrd(); globalOrd != NO_MORE_ORDS; globalOrd = globalOrds.nextOrd()) {
-                                    if (accepted.find(globalOrd) >= 0) {
-                                        continue;
-                                    }
-                                    if (false == acceptedGlobalOrdinals.test(globalOrd)) {
-                                        continue;
-                                    }
-                                    long docCount = bucketDocCount(globalOrd);
-                                    if (bucketCountThresholds.getMinDocCount() == 0 || docCount > 0) {
-                                        consumer.accept(globalOrd, globalOrd, docCount);
-                                        accepted.add(globalOrd);
-                                    }
-                                }
-                            }
-                        }
-                    }
-                }
-            }
-        }
-
         @Override
         public void close() {}
     }
@@ -549,11 +494,9 @@ public void close() {}
      */
     private class RemapGlobalOrds extends CollectionStrategy {
         private final LongKeyedBucketOrds bucketOrds;
-        private final boolean excludeDeletedDocs;
 
-        private RemapGlobalOrds(CardinalityUpperBound cardinality, boolean excludeDeletedDocs) {
+        private RemapGlobalOrds(CardinalityUpperBound cardinality) {
             bucketOrds = LongKeyedBucketOrds.buildForValueRange(bigArrays(), cardinality, 0, valueCount - 1);
-            this.excludeDeletedDocs = excludeDeletedDocs;
         }
 
         @Override
@@ -587,20 +530,27 @@ long globalOrdToBucketOrd(long owningBucketOrd, long globalOrd) {
 
         @Override
         void forEach(long owningBucketOrd, BucketInfoConsumer consumer) throws IOException {
-            if (excludeDeletedDocs) {
-                forEachExcludeDeletedDocs(owningBucketOrd, consumer);
-            } else {
-                forEachAllowDeletedDocs(owningBucketOrd, consumer);
-            }
-        }
-
-        void forEachAllowDeletedDocs(long owningBucketOrd, BucketInfoConsumer consumer) throws IOException {
             if (bucketCountThresholds.getMinDocCount() == 0) {
                 for (long globalOrd = 0; globalOrd < valueCount; globalOrd++) {
                     if (false == acceptedGlobalOrdinals.test(globalOrd)) {
                         continue;
                     }
-                    addBucketForMinDocCountZero(owningBucketOrd, globalOrd, consumer, null);
+                    /*
+                     * Use `add` instead of `find` here to assign an ordinal
+                     * even if the global ord wasn't found so we can build
+                     * sub-aggregations without trouble even though we haven't
+                     * hit any documents for them. This is wasteful, but
+                     * settings minDocCount == 0 is wasteful in general.....
+                     */
+                    long bucketOrd = bucketOrds.add(owningBucketOrd, globalOrd);
+                    long docCount;
+                    if (bucketOrd < 0) {
+                        bucketOrd = -1 - bucketOrd;
+                        docCount = bucketDocCount(bucketOrd);
+                    } else {
+                        docCount = 0;
+                    }
+                    consumer.accept(globalOrd, bucketOrd, docCount);
                 }
             } else {
                 LongKeyedBucketOrds.BucketOrdsEnum ordsEnum = bucketOrds.ordsEnum(owningBucketOrd);
@@ -613,64 +563,6 @@ void forEachAllowDeletedDocs(long owningBucketOrd, BucketInfoConsumer consumer)
             }
         }
 
-        /**
-         * Excludes deleted docs in the results by cross-checking with liveDocs.
-         */
-        void forEachExcludeDeletedDocs(long owningBucketOrd, BucketInfoConsumer consumer) throws IOException {
-            assert bucketCountThresholds.getMinDocCount() == 0;
-            try (LongHash accepted = new LongHash(20, new BigArrays(null, null, ""))) {
-                for (LeafReaderContext ctx : searcher().getTopReaderContext().leaves()) {
-                    LeafReader reader = ctx.reader();
-                    Bits liveDocs = reader.getLiveDocs();
-                    SortedSetDocValues globalOrds = null;
-                    for (int docId = 0; docId < reader.maxDoc(); ++docId) {
-                        if (liveDocs == null || liveDocs.get(docId)) {  // document is not deleted
-                            globalOrds = globalOrds == null ? valuesSource.globalOrdinalsValues(ctx) : globalOrds;
-                            if (globalOrds.advanceExact(docId)) {
-                                for (long globalOrd = globalOrds.nextOrd(); globalOrd != NO_MORE_ORDS; globalOrd = globalOrds.nextOrd()) {
-                                    if (accepted.find(globalOrd) >= 0) {
-                                        continue;
-                                    }
-                                    if (false == acceptedGlobalOrdinals.test(globalOrd)) {
-                                        continue;
-                                    }
-                                    addBucketForMinDocCountZero(owningBucketOrd, globalOrd, consumer, accepted);
-                                }
-                            }
-                        }
-                    }
-                }
-            }
-        }
-
-        private void addBucketForMinDocCountZero(
-            long owningBucketOrd,
-            long globalOrd,
-            BucketInfoConsumer consumer,
-            @Nullable LongHash accepted
-        ) throws IOException {
-            /*
-             * Use `add` instead of `find` here to assign an ordinal
-             * even if the global ord wasn't found so we can build
-             * sub-aggregations without trouble even though we haven't
-             * hit any documents for them. This is wasteful, but
-             * settings minDocCount == 0 is wasteful in general.....
-             */
-            long bucketOrd = bucketOrds.add(owningBucketOrd, globalOrd);
-            long docCount;
-            if (bucketOrd < 0) {
-                bucketOrd = -1 - bucketOrd;
-                docCount = bucketDocCount(bucketOrd);
-            } else {
-                docCount = 0;
-            }
-            assert globalOrd >= 0;
-            consumer.accept(globalOrd, bucketOrd, docCount);
-            if (accepted != null) {
-                accepted.add(globalOrd);
-            }
-        }
-
         @Override
         public void close() {
             bucketOrds.close();