[manager] Run the mutating webhook with configuration, disabling instrumentation CRDs.

[atoulme]

This PR is a draft showing a direction of being able to deploy the mutating webhook using exclusively static configuration rather than instrumentation CRDs.

There are 3 parts to it:

• Add the instrumentation CRD to the config struct, so it can be fulfilled.
• Add an option to disable looking up CRDs for instrumentations, instead relying on what the config says.
• Add an integration test showing this works end to end - this test runs outside of the current e2e tests and performs the installation step explicitly, which is not possible right now with the existing integration tests.

[github-actions]

E2E Test Results

 33 files  ±0  221 suites  ±0   3h 37m 34s ⏱️ - 17m 1s
 85 tests ±0   85 ✅ +2  0 💤 ±0  0 ❌  - 2 
225 runs  ±0  225 ✅ +2  0 💤 ±0  0 ❌  - 2 

Results for commit bea749c. ± Comparison against base commit a62a458.

♻️ This comment has been updated with latest results.

[jaronoff97]

can we not reuse the instrumentation spec from the CRD package?

[atoulme]

you cannot because you get import cycles.

If we choose to have Config define the instrumentation CRD struct, we'd need to move it, and we no longer would generate it from the CRD.

[swiatekm]

This dependency is mostly incidental though. The CRD itself doesn't depend on the config struct, the webhooks do, and they just happen to be in the same package. There's several things we can do to fix this, like moving webhook implementations to a different package, or having them depend on their own config struct rather than the full operator one.

I would also much rather not define the instrumentation spec twice.

[atoulme]

Is it? I assumed the Struct was derived from the CRD generation step. I can look further and break this down into smaller chunks.

[atoulme]

Are the types meant to be exported as Go API for some reason or is it ok to not expose them?

[atoulme]

a very rough cut - moving the types out to an internal package here: #4796 @swiatekm is this in line with what you asked for here? I feel like I miss something.

[jaronoff97]

going to review that new pr and then come back here.

[jaronoff97]

I think I'm confused why we need to embed the instrumentations in the config struct if all we care about is disabling/enabling?

[atoulme]

I think I'm confused why we need to embed the instrumentations in the config struct if all we care about is disabling/enabling?

If we disable the CRD, we can still enable autoinstrumentation. We just define everything in config.

[jaronoff97]

I don't think that makes sense to me – if the CRDs are disabled we shouldn't run autoinstrumentation because we have nothing defined? I don't understand why we want to put the CRDs in the config like this.

[atoulme]

I don't think that makes sense to me – if the CRDs are disabled we shouldn't run autoinstrumentation because we have nothing defined?

No, they are defined in config.