automation and filedescriptorset usage #1

clux · 2021-08-22T12:11:00Z

~~minor tweaks, i'm still struggling to get the servicegenerator to do something, but saw this pattern around and thought, this is probably a good way to sanity assert.~~ had misunderstood servicegenerator (same as you, and had failed to remember). will explore the FDS path.

moves generated protos list output to protos.list (with include_str! -> split)
moves the fds output to protos.fds
using both files as inputs in build.rs to avoid copying anything
adds a POC file appending mechanics at the end of build.rs (committed a comment line for each api)

also moves generated protos list into an input file that i've called protos.list for now. Signed-off-by: clux <sszynrae@gmail.com>

trying just here as well because we are already doing cool stuff with sd and gron. Signed-off-by: clux <sszynrae@gmail.com>

clux · 2021-08-22T15:28:55Z

goofed around a bit with just in here as well to try to order all the scripts in here accordingly:

Signed-off-by: clux <sszynrae@gmail.com>

clux · 2021-08-22T22:11:57Z

Edited a lot and updated. Went with your FileDescriptorSet route, which seems viable. I think we can continue on from something like this and find corresponding types from the api-resources.json and inject as appropriate.

I'm going to be away for a few days. If you want this feel free to squash (to get rid of my failed ServiceGenerator experiments), or fixup. Otherwise I can continue on later in the week (but probably not super actively).

kazk · 2021-08-23T21:25:04Z

build.rs

+ prost_build::Config::new()
+ // should probably switch to this
+ //.btree_map(&["."])
+ .out_dir("./out")


It's fine for now, but we shouldn't output outside OUT_DIR set by cargo in build scripts (out/ was copied from there for reference).

Build scripts may save any output files in the directory specified in the OUT_DIR environment variable. Scripts should not modify any files outside of that directory.
https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script

How should we copy this effectively if we don't hardcode the output path? The cargo build --out-dir flag is unstable (and by the looks of things is unlikely to get stabilised soon), so don't see how we can effectively piggy-back on top of OUT_DIR.

With OUT_DIR, the output happens to be within target/debug/build/k8s-pb-790fe4bcffd42aa5/out/api.core.v1.rs in my case, but it's a bit awkward to grab from. Should we write the "final output" at the end of the build.rs script instead?

I think we should have k8s-pb-codegen binary crate, like k8s-openapi-codegen. This is why I thought codegenrs (#3) might be useful. It seems to make it easy to set this up. I haven't looked into it, though.

Ok, yeah, we definitely need another place to keep most of the logic at the very least. Tried a little bit to build on top of build.rs and the debug flow there is hard. I'll try to look more at it tomorrow 💤

kazk · 2021-08-23T21:29:10Z

build.rs

+ use std::io::Write;
+ if let Some(pkg) = f.package {
+ let pkgpath = std::path::Path::new("./out").join(format!("{}.rs", pkg));
+ let generics = format!("// TODO genericsfor {}\n", pkg);


I think we can continue on from something like this and find corresponding types from the api-resources.json and inject as appropriate.

Yeah, we should be able to get all structs within this package from f.message_type vec.

Have some time to start looking at this properly again now. Matching seems possible, but I might need to restructure the root keys in api-resources.json. The apiGroupVersion seems a little hard to match with f.package (and AFAIKT i don't have a better one in the FileDescriptorSet from a quick debug dump).

justfile

README.md

Remove `gron` dependency and make it easier to maintain patches.

Signed-off-by: clux <sszynrae@gmail.com>

this is just a sketch. i think we definitely need to move this generation code elsewhere. the flow right now is very hard to deal with (script breaks => no output, so have to output into the output files instead). Signed-off-by: clux <sszynrae@gmail.com>

Signed-off-by: clux <sszynrae@gmail.com>

kazk · 2021-10-14T22:38:56Z

https://github.com/influxdata/pbjson (pbjson-build) generates serde::Serialize/serde::Deserialize implementations from FileDescriptorSet. Maybe we can learn something from them.

kazk · 2021-10-27T06:18:29Z

I got spec, status, and condition types for #4 easily, so I'm pushing it here. Hope you didn't have any local changes.

Signed-off-by: kazk <kazk.dev@gmail.com>

Basically a copy of `list-resources.jq` without `verbs` as a map. Should be easier to use for codegen. Signed-off-by: kazk <kazk.dev@gmail.com>

k8s-pb-codegen/openapi/transformed.json

Signed-off-by: kazk <kazk.dev@gmail.com>

kazk · 2021-10-27T19:34:41Z

Can we open a new PR without FileDescriptorSet for now?

With the new transform.jq, Pod looks like this:

{
  "api.core.v1.Pod": {
    "name": "pods",
    "namespaced": true,
    "apiGroupVersion": "v1",
    "group": "",
    "version": "v1",
    "kind": "Pod",
    "proto": "api.core.v1.Pod",
    "rust": "api::core::v1::Pod",
    "metadata": "apimachinery::pkg::apis::meta::v1::ObjectMeta",
    "spec": "api::core::v1::PodSpec",
    "status": "api::core::v1::PodStatus",
    "condition": "api::core::v1::PodCondition",
    "scopedVerbs": {
      "all": [
        "list"
      ],
      "namespaced": [
        "create",
        "delete",
        "deletecollection",
        "get",
        "list",
        "patch",
        "update"
      ]
    },
    "paths": [
      "/api/v1/pods",
      "/api/v1/namespaces/{namespace}/pods",
      "/api/v1/namespaces/{namespace}/pods/{name}"
    ],
    "subresources": [
      {
        "name": "attach",
        "scopedVerbs": {
          "namespaced": [
            "connect"
          ]
        },
        "paths": [
          "/api/v1/namespaces/{namespace}/pods/{name}/attach"
        ]
      },
      {
        "name": "binding",
        "scopedVerbs": {
          "namespaced": [
            "create"
          ]
        },
        "paths": [
          "/api/v1/namespaces/{namespace}/pods/{name}/binding"
        ]
      },
      {
        "name": "ephemeralcontainers",
        "scopedVerbs": {
          "namespaced": [
            "get",
            "patch",
            "update"
          ]
        },
        "paths": [
          "/api/v1/namespaces/{namespace}/pods/{name}/ephemeralcontainers"
        ]
      },
      {
        "name": "eviction",
        "scopedVerbs": {
          "namespaced": [
            "create"
          ]
        },
        "paths": [
          "/api/v1/namespaces/{namespace}/pods/{name}/eviction"
        ]
      },
      {
        "name": "exec",
        "scopedVerbs": {
          "namespaced": [
            "connect"
          ]
        },
        "paths": [
          "/api/v1/namespaces/{namespace}/pods/{name}/exec"
        ]
      },
      {
        "name": "log",
        "scopedVerbs": {
          "namespaced": [
            "get"
          ]
        },
        "paths": [
          "/api/v1/namespaces/{namespace}/pods/{name}/log"
        ]
      },
      {
        "name": "portforward",
        "scopedVerbs": {
          "namespaced": [
            "connect"
          ]
        },
        "paths": [
          "/api/v1/namespaces/{namespace}/pods/{name}/portforward"
        ]
      },
      {
        "name": "proxy",
        "scopedVerbs": {
          "namespaced": [
            "connect"
          ]
        },
        "paths": [
          "/api/v1/namespaces/{namespace}/pods/{name}/proxy",
          "/api/v1/namespaces/{namespace}/pods/{name}/proxy/{path}"
        ]
      },
      {
        "name": "status",
        "scopedVerbs": {
          "namespaced": [
            "get",
            "patch",
            "update"
          ]
        },
        "paths": [
          "/api/v1/namespaces/{namespace}/pods/{name}/status"
        ]
      }
    ]
  }
}

GVK, apiVersion
Name
metadata type for Metadata trait
spec type for HasSpec trait
status type for HasStatus trait
condition type for HasConditions trait
namespaced is true if resource can be namespaced. It can still have all verbs like list.
Scoped resource verbs
Scoped subresource Verbs

clux · 2021-10-27T19:46:00Z

That's definitely a much better json to work with. and have also been sitting on this PR for too long :(

The FileDescriptorSet is only there so we can use the generated json to create impls, and atm it doesn't really do a whole lot; traverses the json path (which is probably wrong now), and appends a bit to the files. Are you suggesting we just comment out all of the FDS stuff from 39 -> 83 for now? Because that makes sense to me.

clux · 2021-10-27T19:48:15Z

Guess there's also the rust structs i wrote for it in lib.rs, but we can drop all of that.

kazk · 2021-10-27T20:02:35Z

traverses the json path (which is probably wrong now), and appends a bit to the files.

I added a separate JSON transformed.json, so it should still work. We can remove the old one in the new PR.

Are you suggesting we just comment out all of the FDS stuff from 39 -> 83 for now? Because that makes sense to me.

I think we should create a new PR with automation and restructuring of the project (a separate codegen crate) that can be merged. Using FDS with transformed JSON is necessary, but it needs much more work, and it's difficult to iterate/collaborate without having the new structure merged first.

kazk · 2021-10-27T20:05:55Z

Or, we can just comment out the code and merge this after cleaning up some commits and fixing DCO.

clux · 2021-10-27T20:07:32Z

I can remove the FDS stuff from this PR quickly if that's ok, then maybe just merge this and I'll open a new PR later with my stashed FDS stuff.

clux · 2021-10-27T20:13:41Z

This commit builds and removes my changes to the generated files (it just maintains the automation): 64ebaa4 if it's ok you can git cherry-pick it in (wasn't able to create a pr into this branch).

kazk · 2021-10-27T20:23:15Z

(wasn't able to create a pr into this branch)

Yeah, that's because this branch is in your fork.

I'll open a new PR later with my stashed FDS stuff.

Sorry if the changes I pushed I messed it up. This is also why I'd like to merge this, so we can open PRs branching from here. I'll cherry pick. Is it ok if I just drop the commits for ServiceGenerator attempts (squash them)?

clux · 2021-10-27T20:25:29Z

Nah, it's fine. I didn't want to impose by pushing straight into this.

But, yeah sounds great. Cherry pick, and rebase drop / squash away what you don't like! Will be great to have this stuff in main!

kazk · 2021-10-27T22:30:01Z

Superseded by #5. Pushed to a new branch just in case I messed up something while rebasing.

add a service generator for build.rs sanity

d121f44

also moves generated protos list into an input file that i've called protos.list for now. Signed-off-by: clux <sszynrae@gmail.com>

clux changed the title ~~add a service generator for build.rs sanity~~ service generator plus script runner Aug 22, 2021

clux force-pushed the protos-work branch from 76bef89 to 9d83b1e Compare August 22, 2021 15:21

move scripts into something that tracks script dependencies

9a31740

trying just here as well because we are already doing cool stuff with sd and gron. Signed-off-by: clux <sszynrae@gmail.com>

clux force-pushed the protos-work branch from 9d83b1e to 9a31740 Compare August 22, 2021 15:25

attempts to make servicegenerator work

80289eb

Signed-off-by: clux <sszynrae@gmail.com>

clux force-pushed the protos-work branch from 267585c to 80289eb Compare August 22, 2021 17:03

clux changed the title ~~service generator plus script runner~~ automation and filedescriptorset usage Aug 22, 2021

drop servicegenerater and use fds to inject into files

1350ff7

Signed-off-by: clux <sszynrae@gmail.com>

clux force-pushed the protos-work branch from c7ebbfb to 1350ff7 Compare August 22, 2021 22:08

kazk reviewed Aug 23, 2021

View reviewed changes

justfile Outdated Show resolved Hide resolved

kazk reviewed Aug 23, 2021

View reviewed changes

README.md Show resolved Hide resolved

kazk added 2 commits August 23, 2021 16:17

Use jq to patch swagger.json

36d8889

Remove `gron` dependency and make it easier to maintain patches.

Add fd to build dependencies list

fdea2e9

kazk force-pushed the protos-work branch from d386aee to fdea2e9 Compare August 23, 2021 23:34

clux added 3 commits September 15, 2021 22:57

force build.rs to rebuild all on just build for now

6c85ca6

Signed-off-by: clux <sszynrae@gmail.com>

move build.rs into a codegen crate

61baaa4

Signed-off-by: clux <sszynrae@gmail.com>

clux self-assigned this Oct 26, 2021

kazk force-pushed the protos-work branch 3 times, most recently from f6695a9 to c4cd3f5 Compare October 27, 2021 06:26

Add spec, status, condition for new traits

e9815c3

Signed-off-by: kazk <kazk.dev@gmail.com>

kazk force-pushed the protos-work branch from c4cd3f5 to e9815c3 Compare October 27, 2021 06:26

kazk added 4 commits October 27, 2021 00:37

Add metadata type

c19076f

Signed-off-by: kazk <kazk.dev@gmail.com>

Add subresources

0fdf919

Signed-off-by: kazk <kazk.dev@gmail.com>

Merge subresources into parent

a932174

Signed-off-by: kazk <kazk.dev@gmail.com>

Transform to map of proto path to resource infos

955920a

Basically a copy of `list-resources.jq` without `verbs` as a map. Should be easier to use for codegen. Signed-off-by: kazk <kazk.dev@gmail.com>

kazk reviewed Oct 27, 2021

View reviewed changes

k8s-pb-codegen/openapi/transformed.json Outdated Show resolved Hide resolved

kazk added 2 commits October 27, 2021 11:26

Sort resources by key

582b9f1

Signed-off-by: kazk <kazk.dev@gmail.com>

Use more consistent formatting and add comments

89a574f

Signed-off-by: kazk <kazk.dev@gmail.com>

kazk mentioned this pull request Oct 27, 2021

Move codegen to a separate crate and use improved transform #5

Merged

kazk closed this Oct 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

automation and filedescriptorset usage #1

automation and filedescriptorset usage #1

clux commented Aug 22, 2021 •

edited

Loading

clux commented Aug 22, 2021

clux commented Aug 22, 2021

kazk Aug 23, 2021

clux Sep 15, 2021

kazk Sep 15, 2021

clux Sep 15, 2021

kazk Aug 23, 2021

clux Sep 15, 2021

kazk commented Oct 14, 2021

kazk commented Oct 27, 2021

kazk commented Oct 27, 2021 •

edited

Loading

clux commented Oct 27, 2021

clux commented Oct 27, 2021

kazk commented Oct 27, 2021

kazk commented Oct 27, 2021

clux commented Oct 27, 2021

clux commented Oct 27, 2021

kazk commented Oct 27, 2021 •

edited

Loading

clux commented Oct 27, 2021 •

edited

Loading

kazk commented Oct 27, 2021

automation and filedescriptorset usage #1

automation and filedescriptorset usage #1

Conversation

clux commented Aug 22, 2021 • edited Loading

clux commented Aug 22, 2021

clux commented Aug 22, 2021

kazk Aug 23, 2021

Choose a reason for hiding this comment

clux Sep 15, 2021

Choose a reason for hiding this comment

kazk Sep 15, 2021

Choose a reason for hiding this comment

clux Sep 15, 2021

Choose a reason for hiding this comment

kazk Aug 23, 2021

Choose a reason for hiding this comment

clux Sep 15, 2021

Choose a reason for hiding this comment

kazk commented Oct 14, 2021

kazk commented Oct 27, 2021

kazk commented Oct 27, 2021 • edited Loading

clux commented Oct 27, 2021

clux commented Oct 27, 2021

kazk commented Oct 27, 2021

kazk commented Oct 27, 2021

clux commented Oct 27, 2021

clux commented Oct 27, 2021

kazk commented Oct 27, 2021 • edited Loading

clux commented Oct 27, 2021 • edited Loading

kazk commented Oct 27, 2021

clux commented Aug 22, 2021 •

edited

Loading

kazk commented Oct 27, 2021 •

edited

Loading

kazk commented Oct 27, 2021 •

edited

Loading

clux commented Oct 27, 2021 •

edited

Loading