ZEP-0048: schema upgrade process

[AustinAbro321]

• One-line PR description: This PR introduces the schema upgrades ZEP

• Issue link: Schema upgrade process #48

[mkcp]

I like the "support for one year" expectation here - it's farther out than we could reasonable expect to tie it to a specific release. Maybe we can have an SDPX tag for the expected deprecation time? Automating it is a bit of scope creep, but I like having the hook there already.

[AustinAbro321]

Yeah we'll need to have some estimation of when we'll drop support, at least in the docs, but I am open to other hooks.

Maybe a warning on zarf package create to a migration docs page that'll give the estimated date to drop support?

[brandtkeller]

Is there criteria or prior art to defining a support period? One year is certainly reasonable - I don't want to back us into a corner where we have baggage to track by having too loose of a policy.

[AustinAbro321]

Not a ton of prior art. When I've deprecated CLI commands in the past I've gone with one year, this is how Kubernetes does it (https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-flag-or-cli). With the SDK we usually give no notice.

I didn't pick the number with a ton of thought. I would be open to other timelines. I think 6 months or 9 months would both probably be reasonable. I wouldn't want to do more than one year.

[Racer159]

Just for Zarf's own historicals it has used CLI version cutoffs for feature removals - https://github.com/zarf-dev/zarf/blob/a26516131a5df8dd2ddc93ec1f2e59bd959c971d/zarf.schema.json#L539 - not saying that is the right way but something to compare against.

[soltysh]

There are 2 important aspects here to distinguish:

1. Reading packages available as source or *.zst files, or just under dev. For those cases it's totally reasonable that we'll stop supporting, will help with migration.
2. Packages that were deployed to the cluster and haven't been touched over that period of time. For those cases we'll very likely will want to consider keeping that read-only capabilities for a bit longer.

[AustinAbro321]

Yeah, I hadn't thought much about deployedPackages. They are used during inspect and remove. Added a line about this that extends deployed packages for another year. We have to keep them around for longer as we don't want to make it impossible for someone to remove an old deprecated package.

[AustinAbro321]

@Racer159 Thanks for calling that out. Added a line for this too, but intent here is for API versions to be decoupled from CLI versions going forward. Only caveat is that we likely won't release Zarf v1 until there is a v1 api version

[mkcp]

TIL schema.json conditionals. Have you explored having versioned schemas instead? That was my initial take on how we'd solve this, not knowing that the conditionals existed. I'm curious about the tradeoffs of the approaches.

[AustinAbro321]

Added some more details, the main reason is for a more convenient UX in text editors, we will probably still create versioned schema's, especially if we end up publishing them in the docs

[mkcp]

Splitting this section up and framing it with code examples of the public facing APIs (CLI, types, funcs) would go a long way here. It doesn't need to be 1:1 what we ship or contain any implementation.

[AustinAbro321]

Agreed, added some detail

[brandtkeller]

When I read this section - I begin to think about a space we've seen some requests - IE interfaces. Have we evaluated an option where each schema implements a common interface? Then the SDK operates consistently across schema versions?

(understanding this adds complexity up front)

[AustinAbro321]

I've thought about it, but I don't think an interface is viable given that the schema is almost exclusively comprised of data rather than functionality. We could make getter functions for every item, but when there are items that are not available across schemas, we'd have to go to the hardcoded types . Additionally when the data being collected is not a primitive type and instead a v1alpha1 / v1beta1 type we would need to shift the interfaces to return different types, would get unwieldy quickly imo.

[soltysh]

Theoretically if you check k8s, it has metav1.Object interface with set of pre-defined getters and setters that are universally available on ALL resources. The question is, could we create such sub-set of fields, and then use that in the interfaces? The obvious downside is that you'll end up casting to specific types, which might hide bugs due to schema migration. But maybe worth at least describing why we decided not to do this.

[soltysh]

The other alternative is creating some sort of internal type, which is a superset of all fields available for any given type, and all the SDK-related operations will only work on those. This would require introducing conversions between external (v1beta1) and internal version. Here the downside mentioned above is not a problem, but then you're leaking internal information. So pick your poison 😅

[AustinAbro321]

I think we could create a subset of fields we know will exist forever, such as the APIVersion and package name. However I think generally an interface like that is more useful when there are a lot of different types of objects whereas in Zarf we have mainly one option with different API versions. I will add something to alternatives for this.

Yeah I discussed the internal type in the alternatives section Public Facing Internal Type. I think the main issue is that it would get confusing both for maintainers and SDK contributors as to which objects on the internal type belong to which package.

[AustinAbro321]

Added a section

[Racer159]

It may be good to canary-in-the-coal-mine this as well - for example if an older version of zarf sees a newer package schema version in a package (even if there is a version it supports) it outputs a warning stating there is a package definition update available.

Deprecation could also be something to represent in the packages themselves so that they can communicate that information to the airgapped Zarf version to throw warnings as well.

[soltysh]

It may be good to canary-in-the-coal-mine this as well - for example if an older version of zarf sees a newer package schema version in a package (even if there is a version it supports) it outputs a warning stating there is a package definition update available.

This is great idea, and could be rather easy to pull off. The only downside is that if someone makes a mistake, zarf can potentially report that v1bbeta1 is available, where in fact there is none 😉

Deprecation could also be something to represent in the packages themselves so that they can communicate that information to the airgapped Zarf version to throw warnings as well.

I've been pondering in my head about how to pull something like that off and I have a hard time to justify this. Because you have to be able to decode the actual yaml, you'll need to have an explicit, well-defined place for defining deprecations, which would mean we'd need to reserve extra field for that. Something like that seems a bit too much, imo.

[AustinAbro321]

This is great idea, and could be rather easy to pull off. The only downside is that if someone makes a mistake, zarf can potentially report that v1bbeta1 is available, where in fact there is none 😉

Yeah added a statement about this. I'm not worried about someone having a bad API version since we'll check that during package create.

Because you have to be able to decode the actual yaml, you'll need to have an explicit, well-defined place for defining deprecations, which would mean we'd need to reserve extra field for that

Yeah we'd have to add a new field, it's doable in the build section, but given that when an API version is deprecated the creator will already get a warning about it and they are the persona responsible for updating, I would agree that it's not something I'd look to add for deployers

[Racer159]

Not all YAML parsers in other languages handle YAML Documents well so it would be worth calling out (if this is implemented as is) that this would be breaking for those implementations.

[soltysh]

How much do we want to care about that case? When someone works with zarf yamls, I'm expecting it to either come from:

1. zarf CLI
2. zarf SDK

Both of which will know how to handle old (single entry) and new (multiple entries) style. I've talked with @AustinAbro321 that we should release the support for multiple yamls asap, even before attempting v1beta1, if that's not there yet.

[AustinAbro321]

I think it is worth calling out in release notes when we make the switch, there might be some users evaluating Zarf packages from other languages that can't call the go sdk. But yeah ultimately, I wouldn't stop this strategy for that use case.

[Racer159]

Yeah my calling it out wasn't to induce a change in the design - more to callout a possible consequence to note in the release (and possibly something to announce early for others to ready their consumption of the new standard). Hopefully anyone else in the community who is relying on Zarf to this extent would also be invested enough in the design process to see this though.

[soltysh]

There are 2 important aspects here to distinguish:

1. Reading packages available as source or *.zst files, or just under dev. For those cases it's totally reasonable that we'll stop supporting, will help with migration.
2. Packages that were deployed to the cluster and haven't been touched over that period of time. For those cases we'll very likely will want to consider keeping that read-only capabilities for a bit longer.

[soltysh]

A few questions, it's pretty clear how this will work with my dev package, but:

1. What about a published package? Example, someone published in a registry packageA, how the above will work with that?
2. What about a deployed package?

[AustinAbro321]

Intent is for this only to work with local zarf.yaml files. Added a line to clarify this

[soltysh]

Theoretically if you check k8s, it has metav1.Object interface with set of pre-defined getters and setters that are universally available on ALL resources. The question is, could we create such sub-set of fields, and then use that in the interfaces? The obvious downside is that you'll end up casting to specific types, which might hide bugs due to schema migration. But maybe worth at least describing why we decided not to do this.

[soltysh]

The other alternative is creating some sort of internal type, which is a superset of all fields available for any given type, and all the SDK-related operations will only work on those. This would require introducing conversions between external (v1beta1) and internal version. Here the downside mentioned above is not a problem, but then you're leaking internal information. So pick your poison 😅

[soltysh]

It may be good to canary-in-the-coal-mine this as well - for example if an older version of zarf sees a newer package schema version in a package (even if there is a version it supports) it outputs a warning stating there is a package definition update available.

This is great idea, and could be rather easy to pull off. The only downside is that if someone makes a mistake, zarf can potentially report that v1bbeta1 is available, where in fact there is none 😉

Deprecation could also be something to represent in the packages themselves so that they can communicate that information to the airgapped Zarf version to throw warnings as well.

I've been pondering in my head about how to pull something like that off and I have a hard time to justify this. Because you have to be able to decode the actual yaml, you'll need to have an explicit, well-defined place for defining deprecations, which would mean we'd need to reserve extra field for that. Something like that seems a bit too much, imo.

[soltysh]

I'm missing a story 4 described from a perspective of a zarf developer wanting to introduce a new API version. That's critical to look at this document through the lenses of a dev, not only users.

[soltysh]

I think you're focusing too much on user-facing changes, and missing the zarf developer story. I think that this particular paragraph has its place, and is needed to cover stories 1-3. But through the rest of the doc I believe you should rather focus more on the internals, how the mechanism will work.

[soltysh]

Does this work? I did a quick google search about it, and I couldn't find reasonable answers 😓

[AustinAbro321]

Yes, I've created a branch to test it out. Also the maru2 uses a similar strategy https://github.com/defenseunicorns/maru2/blob/main/schema.go

[soltysh]

Again, focus more on the internals, and developer story. Since the goal of this particular mechanism is to introduce mechanisms that will be available to devs, the only user-visible outcome should be just that zarf dev upgrade-schema and a bunch of warnings 😉

[soltysh]

There are several pre-requisits that should be met before attempting to ship v1beta1:

1. Ensure that current zarf binary handles list of entries, not a single one.
2. Ensure the logic is complete for conversions. Even if it currently means introducing generic/internal and v1alpha1.

It'll be easier to identify road-blocks even with just the currently available API, and address them ahead of time. I'm not saying that you'll resolve all of them before starting the work on v1beta1, but you should get a lot of the refactoring done prior.