feat: external data mutation

[chewong]

Signed-off-by: GitHub noreply@github.com

What this PR does / why we need it:

Continuation of #1506 (credit to the author @sozercan).

Design docs: https://docs.google.com/document/d/1hPi86jdsCKg8puYT5_s_73mPGExUJeZfyKmvG-XWtPc (credit to the authors @sozercan and @ritazh)

This PR adds external data support to gatekeeper's mutation feature based on the design docs above. It also addresses some PR comments in #1506.

Follow-up items after this PR is merged:

• add documentation and examples to the vNext website
• more e2e scenarios and validation
• restrict provider's URL to https only by defining caBundles in Provider in frameworks

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):

Fixes part of #1576

Special notes for your reviewer:

[codecov-commenter]

Codecov Report

Merging #1891 (8897ad5) into master (2f1046f) will increase coverage by 0.47%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #1891      +/-   ##
==========================================
+ Coverage   53.83%   54.30%   +0.47%     
==========================================
  Files         103      106       +3     
  Lines        9147     9441     +294     
==========================================
+ Hits         4924     5127     +203     
- Misses       3845     3927      +82     
- Partials      378      387       +9     

Flag	Coverage Δ
unittests	54.30% <71.42%> (+0.47%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/mutation/mutators/core/mutation_function.go	58.62% <ø> (-2.67%)	⬇️
pkg/mutation/types/mutator.go	38.88% <ø> (ø)
...pis/mutations/unversioned/zz_generated.deepcopy.go	1.91% <21.42%> (+1.91%)	⬆️
apis/mutations/unversioned/externaldata_types.go	24.00% <24.00%> (ø)
.../mutation/mutators/modifyset/modify_set_mutator.go	51.28% <25.00%> (-0.81%)	⬇️
...mutation/mutators/assignmeta/assignmeta_mutator.go	31.31% <30.00%> (-1.67%)	⬇️
pkg/mutation/mutators/assign/assign_mutator.go	36.58% <46.66%> (-0.61%)	⬇️
apis/mutations/unversioned/value.go	67.30% <77.27%> (-3.75%)	⬇️
pkg/mutation/system_external_data.go	88.97% <88.97%> (ø)
pkg/mutation/system.go	82.11% <90.32%> (-1.33%)	⬇️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f1046f...8897ad5. Read the comment docs.

[seh]

I read the design document, but didn't see this question discussed there. Is it the case that when the "dataSource" field's value is "ValueAtLocation," the mutation can only update the field pointed to by the "location" field? That is, that field's current value is the input to a projection, and we will update only that field's value with the projection's output?

If that's so, it's not possible to use one field's value as input to the external data oracle for the purpose of updating a different field's value. Is that deliberate? I assume that this is related to the way that the current mutation API types preclude reading one field as criteria for updating a different field.

[chewong]

I read the design document, but didn't see this question discussed there. Is it the case that when the "dataSource" field's value is "ValueAtLocation," the mutation can only update the field pointed to by the "location" field? That is, that field's current value is the input to a projection, and we will update only that field's value with the projection's output?

Yes, that's correct.

If that's so, it's not possible to use one field's value as input to the external data oracle for the purpose of updating a different field's value. Is that deliberate? I assume that this is related to the way that the current mutation API types preclude reading one field as criteria for updating a different field.

You can technically pass in a composite value as an input to your external data provider so you can read values from multiple fields. For example, let's say we have the following Assign mutation:

apiVersion: mutations.gatekeeper.sh/v1beta1
kind: Assign
metadata:
  name: demo-privileged
spec:
  applyTo:
  - groups: [""]
    kinds: ["Pod"]
    versions: ["v1"]
  match:
    scope: Namespaced
    kinds:
    - apiGroups: ["*"]
      kinds: ["Pod"]
    namespaces: ["bar"]
  location: "spec.containers[name:foo]"
  parameters:
    assign:
      externalData:
        provider: your-external-data-provider
        dataSource: ValueAtLocation

The container spec of foo container will be sent to your external data provider as a marshaled JSON string. In your provider implementation, you can do the following:

var providerRequest externaldata.ProviderRequest
err = json.Unmarshal(requestBody, &providerRequest)
if err != nil {
	return err
}

for _, key := range providerRequest.Request.Keys {
	container = corev1.Container{}
	err = json.Unmarshal([]byte(key), &container)
	if err != nil {
		return fmt.Errorf("failed to unmarshal container: %w", err)
	}
    // you can access the container here
}

[maxsmythe]

Cool to see work on this!

I did a quick scan to see if I had any high-level thoughts that might change your approach.

[seh]

You can technically pass in a composite value as an input to your external data provider so you can read values from multiple fields.

How far can one push this? Could one pass the entire "spec" field of a Pod and replace it with the result from the data provider?

We have a case where we'd like to add an annotation to an object based on a value from within its "spec" field that we'd pass through a data provider. I take it that's still disallowed, by my reading of the documentation for AssignMetadata.

[maxsmythe]

You can technically pass in a composite value as an input to your external data provider so you can read values from multiple fields.

@chewong This is not true as of the last time I looked at the design doc or in the current implementation:

• The current implementation uses string keys that are sent to the data provider. This is so we can perform deduplication and caching locally (which may be possible in the future with composite values, but raises questions about serialization/type, so sticking to string for now).

• Composite values in mutation are incompatible with the notion of the "placeholder" request parallelization/optimization scheme mentioned above. The reason being that if another mutator can traverse the bounds of the placeholder, then it'd be possible to have placeholders within placeholders, which could not be called in parallel. There are other, more subtle, reasons, but that one's pretty significant.

@seh

We have a case where we'd like to add an annotation to an object based on a value from within its "spec" field that we'd pass through a data provider. I take it that's still disallowed, by my reading of the documentation for AssignMetadata.

Correct, the ability to cross-reference between fields allows for the possibility of infinite recursion.

[chewong]

My latest commit(s) should address all of the PR comments. Feel free to take another look while I am rewriting the unit tests. I've tested it with spec.containers[name:*].image location + sidecar injection (see test.bats) and it works.

[maxsmythe]

Unfortunately I'll be out tomorrow :/ Monday okay?

[maxsmythe]

LGTM, nice job! We probably want to wait on merging this for the sharded rego PR, since that one is blocking release.

[ritazh]

LGTM
Thanks @chewong 🎉

[maxsmythe]

LGTM

[sozercan]

LGTM