Proposal for new core API

[shykes]

Last updated 2022-09-09

This proposal defines a complete Core API for cloak. It is written with the following goals in mind:

1. Feature parity with Dagger 0.2
2. Close to feature parity with Buildkit, with an incremental path to reaching full parity in the future
3. Follow established graphql best practices
4. Solve as many outstanding DX problems as possible

See the README for a complete overview.

[sipsma]

Left comments on bigger things that stuck out to me. There's a lot of misc fields that could be added still (groups, supplementary groups, chroot) and fields that are missing args (i.e. cache mount options, ability to make a mount ro, etc.), but they're small things that fit into the existing patterns, so can save that for later.

Overall LGTM to though, my comments are pretty minor at this point. Having all the withX be in Container rather than Exec makes sense to me and I like that it, for instance, allows mounts to be carried along across multiple execs without respecifying everytime. But if we go this route and it confuses users, I don't think it will be a huge lift to rearrange it, so I'm personally fine with going the route in this PR for now.

[shykes]

I'm ready to call this "good enough" and merge it, so that we can start implementing. Of course we can continue tweaking in follow-up PRs but I think we're close enough to ship this.

wdyt?

[sipsma]

I'm ready to call this "good enough" and merge it, so that we can start implementing. Of course we can continue tweaking in follow-up PRs but I think we're close enough to ship this.

wdyt?

SGTM

[jpadams]

Looks good. I'll be excited to have a single key Directory for FS type things. We had filesystem (in client), directories (in export), and dagger.#FS to keep in mind in 0.2. As well as some places where files, directories, and secrets would get a bit confusing due to disjunction magic.

I continue to get some cognitive dissonance from using a "directory" or "filesystem" to pull a single file string when working with secrets (versus Environment variables which feels like just a string to me, and is), but excited to use this new API and have the single Directory entity.

https://github.com/shykes/cloak/blob/gql-builtins/api/README.md#secrets

From a file: type Directory { secret }

I like that File has been minimized/simplified down to a primitive to use with Directory. I think in my CUE Dagger code I almost always went for #Copy and #Subdir versus #ReadFile/#WriteFile.

[shykes]

I continue to get some cognitive dissonance from using a "directory" or "filesystem" to pull a single file string when working with secrets (versus Environment variables which feels like just a string to me, and is), but excited to use this new API and have the single Directory entity.

Would secretFile or secretFromFile be less confusing to you?

[shykes]

Would secretFile or secretFromFile be less confusing to you?

Or I guess we could move this to File.

Before

query secretFromDir($dir: DirectoryID!, $path: String!) {
  directory(id: $dir) {
    secret(path: $path) {
     id
    }
  }
}

After

query secretFromDir($dir: DirectoryID!, $path: String!) {
  directory(id: $dir) {
    file(path: $path) {
      secret {
        id
      }
    }
  }
}

[jpadams]

Hmmm...yeah, that's tough. You could say: a secret is a special string. Sometimes it acts like a secret environment var or sometimes like a secret file from a directory, but those are really just places/routes where we get the secret from. We shouldn't think of a "secret file" as a "file" that happens to be secret or a "secret env var" as an "env var".

So by that logic, what you have proposed is the best and it's shorter. It also keeps secrets from splitting taxonomically into file-secret and env-secret, etc.

[dolanor]

I have a question regarding withoutX

In the case of directory filtering, having the withoutX set after the initial step, wouldn't it force the creation of a layer that contains the directory that we want to eliminate afterwards?

I guess it's more of an implementation detail, but if it were the case, it could be a problem (no one wants to pass the .git folder of linux around with a giant history when all you want is the checkout workspace)

[shykes]

I have a question regarding withoutX

In the case of directory filtering, having the withoutX set after the initial step, wouldn't it force the creation of a layer that contains the directory that we want to eliminate afterwards?

I guess it's more of an implementation detail, but if it were the case, it could be a problem (no one wants to pass the .git folder of linux around with a giant history when all you want is the checkout workspace)

Good question. In theory buildkit could optimize this before marshaling llb. I don’t know if it does.

[sipsma]

In the case of directory filtering, having the withoutX set after the initial step, wouldn't it force the creation of a layer that contains the directory that we want to eliminate afterwards?

To double check I'm understanding the question, you're saying that it would be more optimal to just not create (or import) the directory in the first place rather than create/import it and then delete it? If so, yes I agree the first is more optimal and should be used whenever possible (i.e. you can use include and exclude arguments when doing a local import to filter paths). But there are situations where it's not possible, e.g. when importing an image or running a command that always creates a directory you want to remove. So it still makes sense to have withoutDirectory to handle those situations.

[shykes]

Hmmm...yeah, that's tough. You could say: a secret is a special string. Sometimes it acts like a secret environment var or sometimes like a secret file from a directory, but those are really just places/routes where we get the secret from. We shouldn't think of a "secret file" as a "file" that happens to be secret or a "secret env var" as an "env var".

So by that logic, what you have proposed is the best and it's shorter. It also keeps secrets from splitting taxonomically into file-secret and env-secret, etc.

I updated the secrets API based on your feedback, it feels cleaner this way.

See host.gql, directory.gql and secret.gql.

I also changed Container { stdout, stderr } to return a File, which in turn can be loaded into a secret. This leaves open the possibility of finding a secure way for eg. the vault extension to load a new secret without writing it to the filesystem. It would require additional implementation work (would need a way to map a File to something that perhaps does not exist in llb to avoid leaking secrets in cache) but it doesn't hurt to have that door open :) cc @aluzzardi @sipsma @kpenfound

[shykes]

I also created the optional ExecOpts input type that we talked about @sipsma @aluzzardi .

Going for lunch break, and will merge this after as a checkpoint, unless there are remaining red flags or strong objections.

Thanks everyone!

[dolanor]

LGTM.

We don't have any way to configure network or CA in this API. Is it because it can come later on? Or we don't plan to deal with that at all?

[shykes]

LGTM.

We don't have any way to configure network or CA in this API. Is it because it can come later on? Or we don't plan to deal with that at all?

It depends on what kind of networking configuration you have in mind.

• There are fields missing in ExecOpts which we can add later, to reach feature parity with llb. Some of those options involve networking, for example hosts.

• One part that IMO we should not add is full control over the container runtime's networking stack: host networking, etc. We need to keep the buildkit host an abstraction to keep the door open to seamless clustering down the road. Note that this remains the same as in Dagger Classic.

• I assume that by CA you mean Certificate Authority? I can't think of any relevant configuration we might offer there, so I may be missing something.

[shykes]

I'm merging this as discussed. Remember, this can keep evolving. But we have a good enough checkpoint that we can start a good amount of implementation work with confidence.