-
Notifications
You must be signed in to change notification settings - Fork 108
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
COMPOSER-2096: Add blueprint support to cloudapi & local access to cloudapi service #3757
Conversation
When discussing this with @supakeen and @achilleas-k, I've also had the idea of making the conversion a separate route. The flow would be basically:
I guess that a small advantage is that the main compose route doesn't get more cluttered, and we don't have to think what has a precedence (one of your comments in the current PR) - if the new blueprint structure, or the old stuff. cc @croissanne |
Forgot to mention: I'm excited to see this being worked on! Thanks, Brian! 🎆 |
That's certainly a possibility, but I think it makes things harder. I really don't see any reason for an extra round-trip of the data, which may also grow considerably in size if we start adding files. If you've sent the data, and it has been consumed by the service, it may as well take action and start the compose. Another alternative is to add another compose endpoint that only takes the full blueprint, and none of the current customizations. I'd prefer that to a 'convert' stage. Especially since, as you can see, on the inside it really is using a blueprint. |
We've spoken about this a bit in the discussion as well but a separate route adds more benefits as it can also be used as a separate linting/checking route and allows (some) shell scripts to get the composerequest separately, store it, and send it out later (something we do in Koji) though they could also 'just submit the blueprint' of course. The files discussion is indeed another can of worms that's going to greatly increase round-trip times. |
Rebased to prevent OCI private key leaking. Apologies :( |
3b04fd9
to
27936d6
Compare
I went ahead and made the customizations and blueprints sections mutually exclusive. It will return an error if you try to use both. I think that we should deprecate customizations and move to using the blueprint section for future features. I've also turned on the local listener for cloudapi under As I see it the goals are to converge on one API, and to maintain support for existing and future blueprints, which this accomplishes. It should be straightforward to add support for this to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really sorry for the late review.
Added 2 comments, but in general I wonder if we can't repurpose the current objects as much as possible in the new blueprint. Even if they require slight tweaks (listening to both "item: []" and "items: []", i think the singular/plural field for arrays is a common difference).
We could do those things, but then we'd lose strict checking of the blueprint in OpenAPI. I think it's better to keep it matching the actual blueprint schema so that we have easy validation of the json as soon as possible. |
487bada
to
434ce4b
Compare
I'd generally agree with this but I can imagine it being frustrating for users of the API to have objects with slightly different keys but are otherwise identical. Maybe this is fine for the API in composer since only our own projects talk to it realistically though. |
True, but the goal is to eventually get rid of the others and have a single API so there should eventually be no confusion :) |
Add a local socket for communicating with the cloudapi. It is started by osbuild-composer.socket and is located at /run/cloudapi/api.socket cloudapi requests can be passed to it using curl like this: curl -k --unix-socket /run/cloudapi/api.socket --header 'Content-Type: application/json' \ --data request.json http://localhost/api/image-builder-composer/v2/compose A simple request.json looks like this: { "distribution": "fedora-38", "image_request": { "architecture": "x86_64", "image_type": "guest-image", "upload_options": {}, "repositories": [ { "name": "fedora", "metalink": "https://mirrors.fedoraproject.org/metalink?repo=fedora-38&arch=x86_64", "check_gpg": false }, { "name": "updates", "metalink": "https://mirrors.fedoraproject.org/metalink?repo=updates-released-f38&arch=x86_64", "check_gpg": false } ] } }
This adds a 'blueprint' section to the compose request. It also restricts it so that only 'blueprint' or 'customizations' can be included, but not both. The goal is to move to using 'blueprint' for all customizations so that there is a single consistent interface for the clients. Where the openapi schemas are the same between the two they have been shared, but a few are different. They are created with 'Blueprint*' as their name. This also re-adds the SSHKey schema removed by commit bfad6d5, it is used by the Blueprint Customization.
If the request includes a blueprint (and not customizations) it uses that blueprint for the compose.
This tests to make sure the blueprint produced by the customizations data and the blueprint data are identical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok let's keep them separate for now. We can always merge objects later where possible, and there's always v3.
The basic idea here is to start by adding blueprint support to cloudapi, the discussion for this can be found here.
This is a really rough poc, but it does actually work, you can submit compose requests to a local socket and it will use the "blueprint" to create it. I only implemented the customizations schema that were the same as a starting point.
Some things to answer:
Schema notes:
Things the same:
Almost the same (name difference):
Field differences:
NOTE: This has no new tests, and CI will most certainly fail. But you can build it with
make scratch
and submit a sample compose request.request-bcl.json: