[Performance] Call AdminDns.isAdmin once per request

[cwperks]

Description

This PR fixes on issue on the GET _mappings API where the security plugin may repeatedly call AdminDns.isAdmin for every single field across all index mappings. The changes in this PR make it so that its called once per request.

• Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Bug fix

Issues Resolved

To be created

Check List

• New functionality includes testing
• New functionality has been documented
• New Roles/Permissions have a corresponding security dashboards plugin PR
• API changes companion pull request created
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[DarshitChanpura]

Change looks good to me. Can we add a test to confirm the before/after of this change?

[codecov]

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.05%. Comparing base (da520a0) to head (094b5d9).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
.../opensearch/security/OpenSearchSecurityPlugin.java	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5752      +/-   ##
==========================================
- Coverage   73.12%   73.05%   -0.08%     
==========================================
  Files         435      435              
  Lines       26665    26671       +6     
  Branches     3999     3999              
==========================================
- Hits        19499    19484      -15     
- Misses       5249     5269      +20     
- Partials     1917     1918       +1     

Files with missing lines	Coverage Δ
...rch/security/configuration/DlsFlsRequestValve.java	0.00% <ø> (ø)
...search/security/configuration/DlsFlsValveImpl.java	64.76% <100.00%> (-0.12%)	⬇️
.../security/privileges/dlsfls/DlsFlsBaseContext.java	100.00% <100.00%> (ø)
.../opensearch/security/OpenSearchSecurityPlugin.java	85.33% <75.00%> (-0.07%)	⬇️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nibix]

Just out of curiousity: Do we know what's the actual bottleneck in isAdmin()? Is it possibly the exception handling and stack unwinding here?

security/src/main/java/org/opensearch/security/configuration/AdminDNs.java

Lines 134 to 143 in da520a0

	public boolean isAdminDN(String dn) {
	if (dn == null) return false;
	try {
	return isAdminDN(new LdapName(dn));
	} catch (InvalidNameException e) {
	return false;
	}
	}

I am wondering if this could be handled diferently, possibly during authc already.

[cwperks]

Just out of curiousity: Do we know what's the actual bottleneck in isAdmin()? Is it possibly the exception handling and stack unwinding here?

It is hitting the catch block for every field. The autocomplete feature of Dev Tools in OSD has been known to cause cluster instability, so this fix would only optimize this to some extent. Dev Tools is still going to degrade in performance for clusters with a very large number of indices + mappings bc Dev Tools tries to load mappings for all indices of the cluster on page load.

I am wondering if this could be handled diferently, possibly during authc already.

yes I think that makes sense. We should only ever call AdminDns.admin once per request.