move projects images to projects tag

[jessfraz]

Sorry about this after generating some stuff I think its better like this :)

[smklein]

Hey, so this is what I was asking about here:

#749 (comment)

I think I still have this question - what makes the cut on where tags go?

[smklein]

ohhh is it like, related to "roles"? like, "people in the project can manage images", rather than specifying "this endpoint talks about images"?

[smklein]

ohhh is it like, related to "roles"? like, "people in the project can manage images", rather than specifying "this endpoint talks about images"?

Actually, this doesn't make much sense to me, because disks and instances both are used as tags for their respective endpoints

[jessfraz]

Honestly I don't have a perscriptive formula yet i figure we will once we try a few things and what works and what doesn't .

But I can tell you how I am using them.

So tags are how the clients are generated so:

Go (https://pkg.go.dev/github.com/oxidecomputer/oxide.go) becomes

client.VPCs.get() // get a vpc where VPC is the main tag 
// OR
client.Instances.listDisks() // where instances in the main tag but we are listing the disks on an instance

Rust (https://docs.rs/crate/oxide-api/0.1.0-rc.31) is similar

client.vpcs().get()
// OR
client.instances().list_disks()

then for the CLI I am using it for generating only the CRUD commands (list, create, edit, delete, etc)

so I pass a macro the tag: https://github.com/oxidecomputer/cli/blob/main/src/cmd_disk.rs#L17

Now anything outside the root CRUD is manual.

in that instances disks to list the disks is a manually written command. Generating these just made it seem like it would be way harder to maintain in the future because the generation code is already really freaking complex

okay so...

lastly the tags determine how the docs are generated see the side bar and sections here:

https://docs.oxide.computer/api

So you can keep that all in mind and that's what I'm trying to do and playing with what is nice and what sucks given that's how all the generation works. Hope that's helps

[jessfraz]

honestly i was debating between this under projects or some sort of new tag for global images.

[jessfraz]

since for example instances arent under projects but also instances dont also have a global counterpart ya know

[jessfraz]

now im almost leaning towards the new tag for global images would make generating the cli a lot nicer too

[jessfraz]

the other thing we could do (cc @ahl) if we have over used tags we could do a string extension to determine various things, obviously none of the generation is set in stone and could easily be changed to read from a different string

[smklein]

If I'm understanding correctly, it seems like the thing we want to be able to differentiate is the "global" vs "non-global" aspect?

This PR, as implemented, does the following:

1. Global images are tagged with images
2. Project-scoped images are tagged with projects
3. Meanwhile, other resources like disks - which are also project-scoped - are tagged with disks

This format doesn't seem that consistent to me - examples (2) and (3) are both project-scoped, but are tagged differently (one refers to the resource, the other refers to the scope). Examples (1) and (3) are both tagged with the name of the resource, but one is global, and the other is project-scoped.
I dunno if this is what you meant by "new tag for global images", but perhaps it makes sense to distinguish between "the resource being acted upon" and "the scope in which the resource is contained"? Something like:

tag = "image"
scope = "global"

vs

tag = "image"
scope = "project"

That being said, all of this seems contingent on where that crud_gen macro is used in the CLI repo, so I suppose if it works for you, it works?

[jessfraz]

yeah for that we could do then an extension on the openapi spec to denote it basically but yes that would work to let me know one is one and one is the other which would be very helpful

none of the other resources have this global counterpart so its really only something that matters now, to your questions of desks versus this

[jessfraz]

I guess roles will also be both so its worth splitting it out versus me just hard coding some custom logic into everything for images

[jessfraz]

lets see what adam thinks too, idk if dropshot should define this extension scope of if dropshot can arbitrarily create extensions on the fly that might be a better route

[jessfraz]

The other thing is we programatically know the scope based on the path right, like anything under /projects etc, so I could code this in like 5 other repos to calculate the scope or we could calculate it at the generation of the spec, regardless it would never have to be manually written if that makes sense which avoids errors

[david-crespo]

I think I'm with Sean — it doesn't feel consistent to me to have the project-scoped images be under the projects tag. Looking through the external endpoints, in almost every case, the tag refers to the resource being fetched/listed, whatever. But I can see how putting both global and project images under the same tag causes a conflict in the method names. I think a global_images tag is a good workaround that avoids complicating the client generators.

[jessfraz]

coool will update, maybe ill call it like "images:global" so I can parse it

[jessfraz]

okay well, if I change it to global_images or global-images or images:global the tests panic:

---- integration_tests::commands::test_nexus_openapi stdout ----
writing temp config: /var/folders/bw/b2wm022n2c35rl5l0f87wmlr0000gn/T/test_commands_config.80676.1
thread 'integration_tests::commands::test_nexus_openapi' panicked at 'expected normal process exit with code 0, got Signaled(6)', nexus/tests/integration_tests/commands.rs:107:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

which makes me think there's some special character check somewhere? cc @ahl

[jessfraz]

that line if you are wondering is just it running --openapi

[jessfraz]

yeah theres a special character check: thread 'main' panicked at 'failed to register entrypoints: Invalid tag: global-images', nexus/src/external_api/http_entrypoints.rs:192:9

[jessfraz]

OH this is me being a dumbass abort

[jessfraz]

okay good to go now

[smklein]

Looks good - I like this version!