Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support running Containerized Composition functions #3465

Merged
merged 44 commits into from Jan 21, 2023
Merged

Conversation

negz
Copy link
Member

@negz negz commented Nov 24, 2022

Description of your changes

Fixes #2524
Closes #3550

This PR implements Composition Functions per the design document.

It introduces a new OCI image - crossplane/xfn - with a corresponding xfn binary.

  • xfn start starts a long-running process, listening on a gRPC API for requests to run .
  • xfn run is a test/helper utility that runs a single Composition Function.

I made a simple no-op Composition Function that can be used to test xfn:

$ cat ~/fnio.yaml | xfn run negz/xfn-nop:latest -

In order to run this Composition Function, xfn will create a new user (and mount) namespace. It will then invoke xfn spark as a user that is root inside the new user namespace, but unprivileged relative to the 'real' parent namespace. Spark will:

  1. Fetch and cache the Composition Function image.
  2. Build an OCI runtime bundle for the Composition Function container run.
  3. Use an OCI runtime (by default crun) to run the Composition Function container.
  4. Clean everything up.

If xfn is run with CAP_SETUID and CAP_SETGID it's able to create a slightly "better" user namespace, in that the user namespace supports up to 65,536 UIDs and GIDs. It's possible to run xfn without any privileges at all, but it will be limited to creating user namespaces where only one UID and GID (the user namespace root's) exist.

If xfn spark is running in an environment that supports overlayfs in user namespaces (i.e. Kernel 5.11 or above, with a cache directory that is not itself on an overlayfs) it will use overlayfs to create container root filesystems. Otherwise, it will fall back to an "uncompressed" image cache that extracts uncompressed tarballs each time a function is run. The overlay is faster, while the uncompressed approach is widely compatible.

This PR also implements the "Crossplane end" of Composition Functions - new Composition logic that is capable of running a pipeline of Composition Functions as well as contemporary Patch & Transform (P&T) style Composition. While this change required significant refactoring of the Composition logic I've tried to keep things mostly the same unless --enable-composition-functions is set.

One notable place this implementation differs from the design document is that Composition Functions must include all desired state. The design document had proposed they could omit desired state they were not concerned with. I couldn't find a good way to make that design work in practice - in particular it wasn't possible to differentiate "delete this resource (or field)" from "I don't have an opinion about this resource (or field)".

Unless anything comes up in review, the only remaining blocker to ship this with v1.11 is writing documentation.

There's a few small things I may attempt to add, In separate PRs, before v1.11 if there is time:

  • Implement image pull auth
  • Have RunFunction return errors with meaningful gRPC status codes
  • Refactor spark.Run for reduced complexity, add tests
  • Figure out broken FunctionIO CRD generation (maybe due to missing object metadata?)

Post v1.11 I would like to:

  • Implement some kind of garbage collection for the image cache
  • Look into optionally deploying xfn as a deployment - it should scale horizontally pretty well.
  • Determine whether we need to do anything to support multi-platform OCI images.
  • Allow functions to access the environment (perhaps once its non-alpha?)
  • Optionally cache results of function runs with the same input for configurable duration

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

This PR needs much more testing, but as of right now Composition Functions are running end-to-end in kind for me. I've also validated that everything appears to work as normal when Composition Functions are not enabled by deploying https://marketplace.upbound.io/configurations/upbound/platform-ref-gcp/v0.3.0.

To test this you can:

  1. Checkout this PR.
  2. Run make to build it
  3. Run ./cluster/local/kind.sh up to get a kind cluster

You can then use the below command to deploy Crossplane with alpha Composition Functions support enabled.

HELM3_FLAGS="--set args={--debug,--enable-composition-functions} --set xfn.enabled=true --set xfn.args={--debug}" ./cluster/local/kind.sh helm-install upbound-system

Once Crossplane is running you can add a functions array to a Composition. I've been using a locally modified fork of platform-ref-gcp to do so.

apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: xgke.gcp.platformref.upbound.io
  labels:
    provider: GCP
spec:
  writeConnectionSecretsToNamespace: upbound-system
  compositeTypeRef:
    apiVersion: gcp.platformref.upbound.io/v1alpha1
    kind: XGKE
  functions:
  - name: nop
    type: Container
    container:
      # This is a function that just returns the desired state that it is passed unmodified
      image: negz/xfn-nop:latest

If you want to try write your own function you can see the I/O specification in the Composition Functions design doc. Note as above, and contrary to the design, that the desired object of a FunctionIO must include all desired state. Your function will be passed a desired object containing the desired state according to Crossplane (e.g. from the resources array of the Composition if any) and any functions that ran before it. Your function may modify the desired state in any way it sees fit.

$ cat ~/fnio.yaml | docker run \
    -i --rm -u nobody \
    --security-opt seccomp=unconfined \
    --mount type=tmpfs,destination=/xfn \
    build-c4cbe24f/xfn-arm64 run negz/xfn-nop:latest -

I've also validated that the above command runs successfully. Note that we don't really have to completely disable seccomp; we just need to allow some extra syscalls such as unshare that are not allowed by Docker's (and containerd's) default seccomp policy.

If folks want to help test things out, here are a few ideas:

  • Make sure everything continues to work the same without the --enable-composition-functions flag.
  • Check that functions can't access the network with networkPolicy: Isolated.
  • Check that functions can access the network (if the runner can) with networkPolicy: Runner.
  • A pipeline of multiple functions.
  • Many XRs using a pipeline of multiple functions.
  • Writing a function.

cluster/images/xfn/Dockerfile Outdated Show resolved Hide resolved
@negz
Copy link
Member Author

negz commented Dec 8, 2022

Here's the example fnio.yaml I tested with:

apiVersion: apiextensions.crossplane.io/v1alpha1
kind: FunctionIO
config:
  apiVersion: database.example.org/v1alpha1
  kind: Config
  metadata:
    name: cloudsql
  spec:
    version: POSTGRES_9_6
observed:
  composite:
    resource:
      apiVersion: database.example.org/v1alpha1
      kind: XPostgreSQLInstance
      metadata:
        name: my-db
      spec:
        parameters:
          storageGB: 20
        compositionSelector:
          matchLabels:
            provider: gcp
      status:
        conditions:
        - type: Ready
          status: True
    connectionDetails:
    - name: uri
      value: postgresql://db.example.org:5432

Note that per https://github.com/negz/xfns/blob/39e6bdb/xfn-nop/nop.sh the negz/xfn-nop:latest function doesn't actually care whether it's passed a FunctionIO, but xfn run does because it currently deserializes and then re-serializes it before passing it to the function's entrypoint.

internal/xfn/container_linux.go Outdated Show resolved Hide resolved
apis/apiextensions/fn/v1alpha1/functionio_types.go Outdated Show resolved Hide resolved
apis/apiextensions/fn/v1alpha1/functionio_types.go Outdated Show resolved Hide resolved
apis/apiextensions/fn/v1alpha1/functionio_types.go Outdated Show resolved Hide resolved
internal/xfn/container_linux.go Show resolved Hide resolved
cmd/xfn/spark/spark.go Show resolved Hide resolved
internal/oci/store/overlay/store_overlay.go Show resolved Hide resolved
cmd/xfn/spark/spark.go Show resolved Hide resolved
internal/xfn/container_linux.go Show resolved Hide resolved
@turkenh
Copy link
Member

turkenh commented Jan 3, 2023

As a side note,

On my M1 Mac, I can successfully run the command in the How has this code been tested with Docker Desktop, however was getting the following error when I use Colima as container runtime:

cat ~/fnio.yaml | docker run \                                                                                  
    -i --rm -u nobody \
    --security-opt seccomp=unconfined \
    --mount type=tmpfs,destination=/xfn \
    build-58577482/xfn-arm64 run negz/xfn-nop:latest -
crossplane: error: function failed: exit status 1: open executable: Permission denied
                   crossplane: error: Running command: /usr/local/bin/crun --root=/xfn/runtime run --bundle=/xfn/c/22c8d322-021e-4c74-9245-c9b72c016313 22c8d322-021e-4c74-9245-c9b72c016313: cannot invoke OCI runtime: exit status 1

With the recent commits error message is as below:

xfn: error: run.Command.Run(): cannot run function: exit status 1: xfn: error: spark.Command.Run(): OCI runtime error: exit status 1

@negz
Copy link
Member Author

negz commented Jan 9, 2023

This PR needs a bunch of cleanup and tests, but as of right now Composition Functions are running end-to-end in kind for me. I haven't had any time to polish so if you want to try it out YMMV, but you should be able to:

  1. Checkout this PR.
  2. Run make to build it
  3. Run ./cluster/local/kind.sh up to get a kind cluster

You can then use the below command to deploy Crossplane with alpha Composition Functions support enabled.

HELM3_FLAGS="--set args={--debug,--enable-composition-functions} --set xfn.enabled=true --set xfn.args={--debug}" ./cluster/local/kind.sh helm-install upbound-system

Once Crossplane is running you can add a functions array to a Composition.

apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: xgke.gcp.platformref.upbound.io
  labels:
    provider: GCP
spec:
  writeConnectionSecretsToNamespace: upbound-system
  compositeTypeRef:
    apiVersion: gcp.platformref.upbound.io/v1alpha1
    kind: XGKE
  functions:
  - name: nop
    type: Container
    container:
      # This is a function that just returns the desired state that it is passed unmodified
      image: negz/xfn-nop:latest

If you want to try write your own function you can see the I/O specification in the Composition Functions design doc. Note that one major way the implementation differs from the proposed design is that the desired object of a FunctionIO must include all desired state. Your function will be passed a desired object containing the desired state according to Crossplane (e.g. from the resources array of the Composition if any) and any functions that ran before it. Your function may modify the desired state in any way it sees fit.

@negz
Copy link
Member Author

negz commented Jan 9, 2023

Here's the remaining TODO list for getting this feature to alpha in v1.11:

Ship blockers

  • Configure composite.Reconciler correctly when both Secret Stores and Comp Fns are enabled
  • Parse and handle FunctionIO results array (e.g. for errors)
  • Refactor PTFComposer to reduce complexity of Compose method
  • Tests for...
    • PTFComposer
    • FallBackComposer
    • overlay.CachingBundler
  • Figure out how to package an OCI runtime with xfn (curl crun fails for most platform/arch combos)

Nice to have

  • Implement image pull auth
  • Move FunctionIO types to apis/apiextensions/fn/io/v1alpha1
  • Have RunFunction return errors with meaningful gRPC status codes
  • Refactor spark.Run for reduced complexity, add tests
  • Figure out broken FunctionIO CRD generation (maybe due to missing object metadata?)

Post v1.11

  • Look into optionally deploying xfn as a deployment - it should scale horizontally pretty well. Would require our gRPC config to have secure (mTLS) communication over TCP and client-side load balancing.
  • Determine whether we need to do anything to support multi-platform OCI images.
  • Allow functions to access the environment (perhaps once its non-alpha?)
  • Optionally cache results of function runs with the same input for configurable duration

@negz negz force-pushed the xfn branch 9 times, most recently from 68c8654 to 44d17cc Compare January 14, 2023 10:55
@negz negz marked this pull request as ready for review January 14, 2023 11:41
@negz negz requested a review from a team as a code owner January 14, 2023 11:41
Unfortunately this seems to be the best and most portable way to get a
relatively recent build of crun.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
This will make sure we have our request ready (successfully) before we
bother forking and execing a new process.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Eventually I'd like this to happen as part of `make reviewable`.

Signed-off-by: Nic Cope <nicc@rk0n.org>
There's no CRD for this type so in practice these are currently ignored.

Signed-off-by: Nic Cope <nicc@rk0n.org>
This isn't yet implemented; I'll re-add when it is.

This commit also addresses a few other small quirks I caught while
reviewing the API types.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Just fixing some strange phrasing, and adding a note about a known issue
with kong.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
Trying to undo a little unimportant drive-by refactoring.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
Just some fixes and clarifications from a self-review pass on the new
functionality.

Signed-off-by: Nic Cope <nicc@rk0n.org>
The initial desired state passed to our function pipeline should
represent only any resources that were rendered by P&T Composition. It
should not include existing composed resources that were rendered by
either the P&T or function pipelines during a previous reconcile unless
they're still being rendered by P&T.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Previously we'd set Rendered: true during P&T Composition, then skip
recording a ref for or applying any resource with Rendered: false.
Resources that originated from the P&T pipeline were never applied,
because they never set Rendered: true.

I've now flipped the logic by switching from a bool to an error, and
updated the field name to clarify we do specifically mean a P&T template
render error, not a general render erro (which could imply an error from
a Composition Function). We don't actually read the error anywhere, but
I'm not worried about the extra memory and recording an error felt more
intuitive than a bool that was false in the happy path (something like
TemplateRenderFailed).

Signed-off-by: Nic Cope <nicc@rk0n.org>
I'd removed this because I forgot it did deepcopy as well as CRD
generation. CRD generation is currently broken for FunctionIO.

Signed-off-by: Nic Cope <nicc@rk0n.org>
I wanted to move to saying "connection detail keys" in FunctionIO to
differentiate from specifically Kubernetes secrets. This is confusing
though, because the old "connection secret key" exists in a lot of
places. It's not strictly wrong; connection details come from a "secret
store" so let's just leave it as is.

Signed-off-by: Nic Cope <nicc@rk0n.org>
… explicitly set

I noticed while fixing this that I'd missed unit tests for most of this
file, so I've now added them.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Copy link
Member

@turkenh turkenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @negz; I believe this will unlock a lot of innovation and take composing infrastructure experience to a whole new level 🚀

I didn't have time to test functionality when the feature was not enabled rather mostly focused on the new functionality. It would be great if we could make sure that things working as before when the flag is not enabled if we haven't done so.

cmd/xfn/spark/spark.go Show resolved Hide resolved
func (o *ConnectionDetailsObserver) ObserveComposedResources(ctx context.Context, s *PTFCompositionState) error {
for _, cd := range s.ComposedResources {
ecfgs := append(ExtractConfigsFromTemplate(cd.Template), ExtractConfigsFromDesired(cd.Desired)...)
e, err := o.details.ExtractConnection(cd.Resource, cd.ConnectionDetails, ecfgs...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good catch; I need to update the template.

internal/xfn/container_linux.go Show resolved Hide resolved
internal/oci/store/overlay/store_overlay.go Show resolved Hide resolved
@negz
Copy link
Member Author

negz commented Jan 20, 2023

I just tested that the network policy works, using a Python Composition Function that annotates all composed resources with a quote from https://api.quotable.io/random - https://github.com/negz/xfns/pull/2/files.

By default I see the following warning emitted by the function when I describe my XR:

  Warning  ComposeResources         9m34s               defined/compositeresourcedefinition.apiextensions.crossplane.io  Cannot get quote: HTTPSConnectionPool(host='api.quotable.io', port=443): Max retries exceeded with url: /random (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0xffff90f3f880>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution'))

However when I update the function to use network policy Runner (i.e. access to the function runner's network) I see my composed resources being annotated. 😄

functions:
- name: quotable
  type: Container
  container:
    image: negz/xfn-quotable:latest
    network:
      policy: Runner
$ k get -o yaml cluster.container|head -n11
apiVersion: v1
items:
- apiVersion: container.gcp.upbound.io/v1beta1
  kind: Cluster
  metadata:
    annotations:
      crossplane.io/composition-resource-name: gke-cluster
      crossplane.io/external-name: platform-ref-gcp-cluster-d2qgn-2g4nq
      quotable.io/author: Only put off until tomorrow what you are willing to die
        having left undone.
      quotable.io/quote: Pablo Picasso

@negz
Copy link
Member Author

negz commented Jan 20, 2023

I didn't have time to test functionality when the feature was not enabled rather mostly focused on the new functionality. It would be great if we could make sure that things working as before when the flag is not enabled if we haven't done so.

I completely agree. I've run some cursory tests (i.e. using https://marketplace.upbound.io/configurations/upbound/platform-ref-gcp/v0.3.0 and making sure everything works normally). I think we need more, but I'm inclined to merge this PR so that we can ask the community to make sure everything still works in master before we cut a release in a little over a week.

I don't think this will make anything more secure, but it should at
least make sure we're not passing a needlessly complex path around.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Custom Compositions
3 participants