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

RFC: tweak the way customization are done #55

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented May 7, 2024

This is a complex RFC, sorry, I can try to break it into smaller discussion points but it's tricky. Let me try to break it down:

  1. Simplify the way customization inputs are read. it seems we are already doing yaml everywhere so let's just read an input file (just like a blueprint today with composer). Passing the values via -Ckey=value seems cumbersome and we (or our users) will run into quoting issues. Or is there a benefit of doing it that I'm missing? Note that we could simply support -C - to read the data from stdin if scriptability is the goal.

  2. Describe why otk.define and otk.customization are distinct. I don't have a good answer here but I think it's quite important that we have clarify why we need a different mechanism. On the surface it looks like another way to define a variable (which we can do today) and some if/default which we could add to otk.defined variables (fwiw, I'm not fully clear why we need two but my gut-feeling is that it's the right approach so having it slightly more formal/well defined "why" would be nice).

  3. Tweak how the customizations recieve their data. The existing strawman was this. which is a reasonable choice, this commit uses $customizationname now directly, I'm not sure what is the best but it looks nice in the examples so maybe it's a reasonable starting point.

  4. Tweak the example - this is a tricky one. So far in our actual examples we have very simple cases (hostname, timezone) etc which are just strings (and which we could support without a otk.customization by just defining a default otk.define.hostname and then letting the user override it via a otk.include "user-customizations". When trying to build a complex example with the user customization or the container customization I ran into the issue that both have multiple entries so writing them directly in the yaml is tricky. It seems users would become a otk.external.users so that we can use python to generate the required stages. But I think it would be good to find examples of complex customizations so that we nail the concept a bit more down.

@@ -196,26 +196,66 @@ otk.meta.kiwi:

## `otk.customization`

A `otk.customization` is distinct from a `otk.define` because:
- End-users will supply them
Copy link
Member

Choose a reason for hiding this comment

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

We want to be able to inspect omnifests to see which customizations are allowed; we want to provide default values for customizations (when not enabled) which we don't support in defines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both could be done without a new concept (not saying we should, my gut feeling is that we want customizations but I think it's important to figure out why exactly), we could have in our default

otk.define:
 customizations.hostname: "localhost.localdomain"

and then the user provides "config.yaml" with:

otk.define:
  customizations.hostname: "my.hostname"

We could know what customizations are allowed via convention (anyhting defined via otk.define with "customization.*" is a customization).

Again, not saying we should do this but I'm still struggling to have a clear picture in my head.

is passed data. If a customization is passed multiple times then the `defined`
block is repeated multiple times, once for each input.
a `customization` file `otk compile -C customizations.yaml` that contains a
map of customization settings in yaml format, e.g.
Copy link
Member

Choose a reason for hiding this comment

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

You would have to specify this format further. Customizations should be able to be passed multiple times (e.g: files).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, we could allow multple -C that are merged (just like the previous specification). Whats the concern about:

file:
 - path: /path/1
   user: ...
 - path: /path/2

i.e. having them in the same customization.yaml ? Lack of flexibility?

Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

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

I've added some reasons as to why customizations are separate. I will also say that I am not a huge fan of creating a separate file for customizations like this.

If I were to want to run otk from the command line I'd like to do it entirely over stdin/stdout (e.g: otk compile -Chostname=value -Ccomplex='{"foo": "bar"}' - < omnifest.yaml > manifest.json

This is a complex RFC, sorry, I can try to break it into smaller
discussion points but it's tricky. Let me try to break it down:

1. Simplify the way customization inputs are read. it seems we
are already doing yaml everywhere so let's just read an input
file (just like a blueprint today with composer). Passing the
values via `-Ckey=value` seems cumbersome and we (or our users)
will run into quoting issues. Or is there a benefit of doing it
that I'm missing? Note that we could simply support `-C -` to
read the data from stdin if scriptability is the goal.

2. Describe why `otk.define` and `otk.customization` are distinct.
I don't have a good answer here but I think it's quite important
that we have clarify why we need a different mechanism. On the
surface it looks like another way to define a variable (which we
can do today) and some `if/default` which we could add to
`otk.define`d variables (fwiw, I'm not fully clear why we need
two but my gut-feeling is that it's the right approach so having
it slightly more formal/well defined "why" would be nice).

3. Tweak how the customizations recieve their data. The existing
strawman was `this.` which is a reasonable choice, this commit
uses `$customizationname` now directly, I'm not sure what is the
best but it looks nice in the examples so maybe it's a reasonable
starting point.

4. Tweak the example - this is a tricky one. So far in our actual
examples we have very simple cases (hostname, timezone) etc which
are just strings (and which we could support without a `otk.customization`
by just defining a default `otk.define.hostname` and then letting
the user `override` it via a `otk.include "user-customizations"`.
When trying to build a complex example with the `user` customization
or the `container` customization I ran into the issue that both
have multiple entries so writing them directly in the yaml is
tricky. It seems `users` would become a `otk.external.users` so
that we can use python to generate the required stages. But I
think it would be good to find examples of complex customizations
so that we nail the concept a bit more down.
@mvo5
Copy link
Contributor Author

mvo5 commented May 8, 2024

I've added some reasons as to why customizations are separate. I will also say that I am not a huge fan of creating a separate file for customizations like this.

If I were to want to run otk from the command line I'd like to do it entirely over stdin/stdout (e.g: otk compile -Chostname=value -Ccomplex='{"foo": "bar"}' - < omnifest.yaml > manifest.json

This works great for small customizations like "hostname" etc. But once the customizations get more complex it feels a bit unwieldy and mostly for automatically generated content. There is also the issue that complex customizations may create files and then we could run into the commandline ARG_MAX limit (which is big, 2mb so maybe not really an issue).

Also I would argue that customizations are an end-user concept so simplicity and symmetry are important. Doing them on the commandline would mean using json which is not symmetric with the rest of otk (yaml) and most users will probably write them to a file where they get syntax highlighting and can reuse them easier.

We can still support the full comamndline use-case filename with e.g.

$ otk compile -C<(echo '{"foo": "bar"}') - < omnifest.yaml > manifest.json

or

$ otk compile -C- omnifest.yaml > manifest.json <<<EOF
{"foo": "bar"}
EOF

(and we can/should document this :)

But maybe I'm missing something here, maybe it's adventitious for composer in some way?

@mvo5
Copy link
Contributor Author

mvo5 commented May 14, 2024

Some important arguments came up during a virtual discussion:

  1. having json schema validation for the possible customization options is desired which would not work for "otk.defines" (at least not as is, might be useful there too but feels strange)
  2. otk.define will stop working once customizations go beyond simple options like "language", i.e. the openscap tailoring will require adding osbuild stages when enabled so we need to see how that fits

The consensus is to look at more complex customization examples soon. For (2) there is still the option of jinja2 and simple otk.define but it's most likely unwiedly soon (but we need to try).

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.

None yet

2 participants