Bulk API Keys update

[michalpristas]

What is the problem this PR solves?

Instead of creating and invalidating API keys with every policy change it updates them.
On top of that this PR modifies bulker so updates are grouped by role applied.

• When agent is enrolled api key is created
• with every policy change api key is updated
• when agent is unenrolled api key is invalidated

Bulker contains new queue where requests contains ID of an agent and proposed role descriptors.
On flush logic goes through the queue and composes IDs per roles. then it makes a single request to ES.
In case content size of this request is larger than predefined value. it splits IDs to make it suitable. value is configured and is defaulting to 100MB as configured in ES.
Grouping is not that optimal as I would like but we need to keep in mind and make sure that if 2 changes for one ID are in a queue only latest is applied.

the tricky thing is to clean roles on ACK as we don't have a way of persisting desired roles.
How this PR approaches the problem is that instead of persisting roles in agent structure it marks them stale in persisted API Key

e.g we remove network integration, we ended up with api key containing two sets of roles. set of roles is a map with guid as a key
e.g

"497ac86b-40ac-49ce-88d6-dbdb7ac449d4": {
			"applications": [],
			"cluster": [],
			"indices": [{
				"allow_restricted_indices": false,
				"names": ["logs-mongodb.log-default"],
				"privileges": ["auto_configure", "create_doc"]
			}, {
				"allow_restricted_indices": false,
				"names": ["metrics-mongodb.collstats-default"],
				"privileges": ["auto_configure", "create_doc"]
			}, {
				"allow_restricted_indices": false,
				"names": ["metrics-mongodb.dbstats-default"],
				"privileges": ["auto_configure", "create_doc"]
			}, {
				"allow_restricted_indices": false,
				"names": ["metrics-mongodb.metrics-default"],
				"privileges": ["auto_configure", "create_doc"]
			}, {
				"allow_restricted_indices": false,
				"names": ["metrics-mongodb.replstatus-default"],
				"privileges": ["auto_configure", "create_doc"]
			}, {
				"allow_restricted_indices": false,
				"names": ["metrics-mongodb.status-default"],
				"privileges": ["auto_configure", "create_doc"]
			}],
			"metadata": {},
			"run_as": [],
			"transient_metadata": {
				"enabled": true
			}
		},

when removing mongo we create a desired role description set and add this one with modified key by appending -rdstale also to avoid naming conflicts. sometimes content of this changes and we ended up with reduced descriptor set (e.g when disabling monitoring.logs but keepiing monitoring.metrics or disabling metrics for mongo integration)

then on ack we iterate through api key keys and leave out everything with rdstale suffix

To test this you need latest ES. We are using new Update API and modified GET apikey which also returns role descriptions. this allows us not to persist roles and rely on ES.

How to test this PR locally

Checklist

• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

Closes #1581

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-09-20T06:46:33.177+0000

• Duration: 10 min 27 sec

Test stats 🧪

Test	Results
Failed	0
Passed	304
Skipped	1
Total	305

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

[jlind23]

@michalpristas does it relates #1581 or should it close it?

[michalpristas]

@jlind23 this is exactly it, i also linked it as a related issue

[cmacknz]

Have you thought about how to scale test this? Are you able to measure that this is more efficient?

[cmacknz]

Where do these numbers come from? How would we know if they need to change?

[michalpristas]

i took api key and added some buffer.
but knowing api key size i can do this dynamically, good point

[michalpristas]

not sure how to approach scale tests.
as this is happening per checkin we would notice just a small difference in checkin time.
we have nothing to compare in regards create/update as before it was split per checkin now it's bulked into a single operation.
what we could measure is CPU load of ES when dealing with policy change or how long it takes all agents to be reported as up to date.

[mergify]

This pull request is now in conflicts. Could you fix it @michalpristas? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b feat/bulk-api-update upstream/feat/bulk-api-update
git merge upstream/main
git push upstream feat/bulk-api-update

[joshdover]

not sure how to approach scale tests.
as this is happening per checkin we would notice just a small difference in checkin time.
we have nothing to compare in regards create/update as before it was split per checkin now it's bulked into a single operation.
what we could measure is CPU load of ES when dealing with policy change or how long it takes all agents to be reported as up to date.

Yes I believe we should expect to see much lower ES CPU usage, especially during bulk policy reassignments. Unfortunately, I don't believe we yet have a way to build a custom elastic-agent image to use in a scale test before we merge (this is something I will follow up on solving).

I think we should just try to merge this and take a look at how it impacts scale tests once it's in 8.5 snapshots. If there's an issue we can revert this.

[mergify]

This pull request is now in conflicts. Could you fix it @michalpristas? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b feat/bulk-api-update upstream/feat/bulk-api-update
git merge upstream/main
git push upstream feat/bulk-api-update

[AndersonQ]

[Blocker]
Please don't log an error as info, it's confusing and misleading. If it isn't critical, use warning then.

[AndersonQ]

[Blocker]
same as above

[AndersonQ]

[Blocker]
Same as above

[AndersonQ]

[Suggestion]
Keep it consistent, if the method is update, use the same term in the log

Suggested change

	zlog.Info().Err(err).RawJSON("roles", clean).Str("id", apiKeyID).Msg("Failed to refresh API Key")
	zlog.Info().Err(err).RawJSON("roles", clean).Str("id", apiKeyID).Msg("Failed to update API Key")

[AndersonQ]

Why not apiKeyID or api.key.id?

[AndersonQ]

[Blocker]
Again, please avoid 2 different logs with the same message

[AndersonQ]

You could let just the for, no need for this first try, besides the for is trying %s-0-rdstale again