Initial trivial hello-world entitlements agent #113112

prdoyle · 2024-09-18T13:20:57Z

The agent just calls a method to announce it has booted.

elasticsearchmachine · 2024-09-18T13:21:33Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

rjernst

Why do we need 2 jars? Isn't the agent jar effectively on the boot classpath?

rjernst · 2024-09-18T13:30:26Z

distribution/tools/entitlements-agent/build.gradle

+
+test {
+  dependsOn(jar)
+  jvmArgs "-javaagent:${jar.archiveFile.get().asFile}"


I don't think this will work. The string interpolation will happen at gradle configuration time, but it needs to be delayed to runtime. In the past I've done hacks with GString like "${-> '-javaagent:' + jar.archiveFile.get().asFile}, but I'm sure @mark-vieira has a better way to do that lazy evaluation now.

Is this supposed to start ES with the agent? If so, how we would do that?
I think it's a good idea to add a minimal README.md (just a few lines atm with the basic purpose of this entitlements-agent project and how to launch it).

Does it need to be delayed to runtime if it's only for the agent's own unit tests?

@rjernst - can I merge it like this? Or does it need to be changed?

@breskeby - is this something you know about?

you applied my suggestion and I verified the fix locally seeing the agent is correctly applied to the tests and we didn't break task avoidance: https://gradle-enterprise.elastic.co/s/q7l56qcrj2cbm/performance/configuration/by-project?openProjects=WyJwcm9qZWN0LTpkaXN0cmlidXRpb246dG9vbHM6ZW50aXRsZW1lbnQtYWdlbnQiXQ#project-:distribution:tools:entitlement-agent
👍

can I merge it like this? Or does it need to be changed?

Two jars seem like an unnecessary distinction to me. But I'm ok with merging as-is and debating that separately.

rjernst · 2024-09-18T13:33:32Z

...ents-runtime/src/main/java/org/elasticsearch/entitlements/runtime/api/EntitlementChecks.java

+public class EntitlementChecks {
+    static boolean agentSaidHello = false;
+
+    public static void helloFromAgent() {


can we make this not public? It should be an impl detail?

I also would prefer it not be "hello" (sounds like a toy project), but something that can go into production like "agent booted".

If I selectively export it to the agent, then public is ok, right?

libs/entitlements-runtime/src/main/java/module-info.java

prdoyle · 2024-09-18T13:45:01Z

@rjernst - The idea was to separate the agent code from the runtime permission-checking code. The latter is always required to be present in a running Elasticsearch process, even if we do ahead-of-time instrumentation, but the former is only required if we're using an agent.

ldematte

Do we want to have a basic IT test that asserts that ES starts correctly with the agent? This will give us the IT scaffolding + a quick sanity check things are working (and keep working).
But I'm fine adding this in this PR or a separate PR.

ldematte · 2024-09-18T13:51:32Z

distribution/tools/entitlements-agent/build.gradle

+
+test {
+  dependsOn(jar)
+  jvmArgs "-javaagent:${jar.archiveFile.get().asFile}"


Is this supposed to start ES with the agent? If so, how we would do that?
I think it's a good idea to add a minimal README.md (just a few lines atm with the basic purpose of this entitlements-agent project and how to launch it).

prdoyle · 2024-09-18T13:54:40Z

@ldematte - no, that makes the agent module start its own unit tests with the agent attached. I don't have a test for ES booting with the agent.

ldematte · 2024-09-18T14:22:56Z

I don't have a test for ES booting with the agent.

I was referring more at a gradle :run option to start ES with the agent (e.g. an additional option to RunTask to do something like gradle :run --with-entitlements-agent or similar.
Or any other way to start ES with the new agent added to the jvm args
@mark-vieira what's the best way to do that? Can we just pass options through (like we do with --debug-jvm) or do we need changes to JvmOptionsParser and related?

ldematte · 2024-09-18T14:41:01Z

(I'm OK with adding that in a follow up PR too - I'd prefer slightly to do it now so this PR is nice and well-contained, but either way works for me. Same for the IT test(s), I have a slight preference to have them added now but you can raise another PR later).

jdconrad

@prdoyle This is looking really good, thank you! One minor comment. I will leave it to others to approve as I don't have much expertise in the build process related to gradle.

...ution/tools/entitlements-agent/src/main/java/org/elasticsearch/entitlements/agent/Agent.java

ldematte · 2024-09-19T07:00:43Z

Just writing this down here so we don't forget: besides an option to run it manually, it would be great if we can have a flag or something to run the whole CI tests with the agent enable. Of course, optionally!
Then we can ask delivery if we can have a periodic task or something (a github tag?) that runs e.g. part1 with the agent, to know early if we are breaking things.
This is definitely a follow-up PR though! Maybe even a separate task.

prdoyle · 2024-09-19T12:32:52Z

It won't be long before we want our agent always running in all tests, and even in production.

ldematte

LGTM!
As we agreed offline, the additional stuff (gradle command, IT or QA tests, etc.) we discussed here will all go in follow-up PRs.

distribution/tools/entitlement-agent/build.gradle

Co-authored-by: Rene Groeschke <rene@breskeby.com>

distribution/tools/entitlement-agent/build.gradle

Co-authored-by: Rene Groeschke <rene@breskeby.com>

distribution/tools/entitlement-agent/build.gradle

Co-authored-by: Rene Groeschke <rene@breskeby.com>

distribution/tools/entitlement-agent/build.gradle

Co-authored-by: Rene Groeschke <rene@breskeby.com>

breskeby

I suggested some changes to the gradle parts of that PR

* Initial hello-world entitlements agent * Respond to Ryan's comments * License header * Fix forbidden APIs setup * Rename EntitlementAgent * Automated refactor missed one * Automated rename really let me down here * Very serious test name * README files for the new modules * Use "tasks.named('jar')" Co-authored-by: Rene Groeschke <rene@breskeby.com> * Use 'tasks.named('test')' Co-authored-by: Rene Groeschke <rene@breskeby.com> * More deferral of gradle tasks Co-authored-by: Rene Groeschke <rene@breskeby.com> * Even more deferral Co-authored-by: Rene Groeschke <rene@breskeby.com> * FIx gradle syntax for javaagent arg --------- Co-authored-by: Rene Groeschke <rene@breskeby.com>

* Initial trivial hello-world entitlements agent (#113112) * Initial hello-world entitlements agent * Respond to Ryan's comments * License header * Fix forbidden APIs setup * Rename EntitlementAgent * Automated refactor missed one * Automated rename really let me down here * Very serious test name * README files for the new modules * Use "tasks.named('jar')" Co-authored-by: Rene Groeschke <rene@breskeby.com> * Use 'tasks.named('test')' Co-authored-by: Rene Groeschke <rene@breskeby.com> * More deferral of gradle tasks Co-authored-by: Rene Groeschke <rene@breskeby.com> * Even more deferral Co-authored-by: Rene Groeschke <rene@breskeby.com> * FIx gradle syntax for javaagent arg --------- Co-authored-by: Rene Groeschke <rene@breskeby.com> * Entitlements for System.exit (#114015) * Entitlements for System.exit * Respond to Simon's comments * Rename trampoline -> bridge * Require exactly one bridge jar * Use Type helpers to generate descriptor strings * Various cleanup from PR comments * Remove null "receiver" for static methods * Use List<Type> instead of voidDescriptor * Clarifying comment * Whoops, getMethod * SuppressForbidden System.exit * Spotless * Use embedded provider plugin to keep ASM off classpath * Oops... forgot the punchline * Move ASM license to impl * Use ProviderLocator and simplify bridgeJar logic * Avoid eager resolution of configurations during task configuration * Remove compile-time dependency agent->bridge --------- Co-authored-by: Mark Vieira <portugee@gmail.com> * Initial InstrumenterTests (#114422) * Initial InstrumenterTests * Assert on instrumentation method arguments --------- Co-authored-by: Rene Groeschke <rene@breskeby.com> Co-authored-by: Mark Vieira <portugee@gmail.com>

Initial hello-world entitlements agent

a5f15c1

prdoyle requested review from a team as code owners September 18, 2024 13:20

prdoyle added >non-issue :Core/Infra/Core Core issues without another label labels Sep 18, 2024

prdoyle requested review from ldematte and jdconrad September 18, 2024 13:21

elasticsearchmachine added Team:Core/Infra Meta label for core/infra team v9.0.0 labels Sep 18, 2024

rjernst reviewed Sep 18, 2024

View reviewed changes

ldematte reviewed Sep 18, 2024

View reviewed changes

prdoyle added 3 commits September 18, 2024 09:56

Respond to Ryan's comments

d36f17f

License header

bfdc0c8

Fix forbidden APIs setup

6c05a5c

prdoyle requested review from ldematte and rjernst September 18, 2024 14:21

jdconrad reviewed Sep 18, 2024

View reviewed changes

...ution/tools/entitlements-agent/src/main/java/org/elasticsearch/entitlements/agent/Agent.java Outdated Show resolved Hide resolved

Rename EntitlementAgent

4d89a03

prdoyle added 5 commits September 19, 2024 08:34

Automated refactor missed one

d9b239b

Automated rename really let me down here

2c2248c

Very serious test name

4bc69cc

Merge remote-tracking branch 'upstream/main' into trivial-agent

004f42b

README files for the new modules

2c2c1bb

ldematte approved these changes Sep 20, 2024

View reviewed changes