ZEP-0051: v1beta1 schema

[AustinAbro321]

• One-line PR description: This proposes the v1beta1 schema

• Issue link: v1beta1 schema #51

[brandtkeller]

Minor comments to an overall comprehensive update.

[brandtkeller]

Are we expecting any change in behavior for the merge strategies behaviors?

Specifically now that images are a list of objects - we should have the ability to override on collision now.

[AustinAbro321]

Added a line in the proposal next to the "- .components.[x].images" block.

[brandtkeller]

perfect. In doing so have we also removed the previous limitation of images templating on imports?

[AustinAbro321]

Which limitation are you referring to? Remote components? In that case, no, remote components must be templated before publish / import.

[brandtkeller]

As part of scope for zarf-dev/zarf#4491 - wherein there may be issues with skeletons and templates around publishing. Given both the requirement for templating occurring BEFORE publish and the ability to negate needing to template at all with better merge behaviors.

[AustinAbro321]

Yeah #4491 will no longer be solved by the v1beta1 changes

[soltysh]

Suggestion: if you're considering being more open to non-golang consumers, I'd suggest to switch this to an integer expressing number of seconds, you could even go as far as renaming it to timeoutSeconds. This way you're not requiring them to implement ParseDuration-compatible functionality.

[AustinAbro321]

hmm, in that case we could probably just keep it as maxTimeoutSeconds since that's what we're changing here. Has Kubernetes started using seconds instead of parse duration functionality for this reason? I don't think users will attempt to use this field outside of Zarf, but I do want us to be consistent throughout the schema

[soltysh]

Variant and Component are overly complicated imo. Currently, you can end up with something like:

apiVersion: zarf.dev/v1beta1
kind: ZarfComponentConfig
componentA:
    ...
variants:
    - componentB...
      only...
    - componentB...
      only...

In other words, you have multiple levels of indirection. Wouldn't it be better to just move variants to be top-level in that case? This way you'd either have a single variant (with or w/o only), or multiple. So something like this:

apiVersion: zarf.dev/v1beta1
kind: ZarfComponentConfig
variants:
    - componentA...
    - componentB...
      only...
    - componentB...
      only...

That should read better. In general I'd advise designing API such that there's only one way to specify things, rather than multiple.

[AustinAbro321]

I added the oneof_required key to the schema to make this more clear, but the idea is you would only specify either component or variants. You would not be allowed to specify both

The example yaml below wouldn't work since a ZarfComponentConfig will only be allowed to specify a single component. It wouldn't be allowed to have both componentA and componentB

apiVersion: zarf.dev/v1beta1
kind: ZarfComponentConfig
variants:
    - componentA...
    - componentB...
      only...
    - componentB...
      only...

We could change it to have only a components block (the name variants would also work), then validate that only a single component has been defined, however it felt strange to have a list components when it will usually be one option. I was 50/50 on this though, and had already written an alternative on it. LMK what you think given this context

[soltysh]

Only is not the best name to express the desire. Maybe something like target or filter would better express that desire.

[soltysh]

Also, while you're at it, currently ZarfComponentOnlyTarget looks like so:

type ZarfComponentOnlyTarget struct {
	// Only deploy component to specified OS.
	LocalOS string `json:"localOS,omitempty" jsonschema:"enum=linux,enum=darwin,enum=windows"`
	// Only deploy component to specified clusters.
	Cluster ZarfComponentOnlyCluster `json:"cluster,omitempty"`
	// Only include this component when a matching '--flavor' is specified on 'zarf package create'.
	Flavor string `json:"flavor,omitempty"`
}

Why do we need all of those fields? Specifically, why we separately define LocalOS and Cluster? Why we can't have something as simple as:

type ZarfComponentOnlyTarget struct {
	// Only create and deploy to clusters of the given architecture.
	Architecture string `json:"architecture,omitempty" jsonschema:"enum=amd64,enum=arm64"`
	// A list of kubernetes distros this package works with (Reserved for future use).
	Distros []string `json:"distros,omitempty" jsonschema:"example=k3s,example=eks"`
	// Only include this component when a matching '--flavor' is specified on 'zarf package create'.
	Flavor string `json:"flavor,omitempty"`
}

[AustinAbro321]

Yeah, I like target better, updated to move only->target

[AustinAbro321]

I am realizing that Distros is a field that is not used. It was reserved for future use years ago. I am adding it to removed fields.

[AustinAbro321]

localOS and .cluster.architecture cover two different things. localOS decides whether a component deploys on a windows machine or linux machine for instance. .Cluster.Architecture looks at the architecture of the package.

I did drop the cluster verbiage and in line this .cluster. We never check the cluster's node architecture, this is soley based on the architecture that the package is being built towards

[soltysh]

If you want to shorten the names, I'd suggest cutting the Zarf prefixes from all these structs. They are part of a zarf package, so when you're looking in the code at zarfv1beta1.Metadata that should be sufficient. Also Metadata - I'd expect be identical for both the Package and Component, not separate.

[AustinAbro321]

Yeah that makes sense. Updated the structs

[AustinAbro321]

I didn't include the fields Uncompressed and Architecture because ZarfComponentConfigs can't be built on their own, so it's not possible for them to be compressed or have a specific architecture

[soltysh]

I'd love to see the whole API as a set of structs, from top-level ones to more detailed ones. Such that I can have a look at the top-level and see that we have:

type ZarfPackage struct {
    // standard object metadata
    Metadata `json:",inline"`
    
    // List of components to deploy in this package.
	Components []ZarfComponent `json:"components" jsonschema:"minItems=1"`

   // .... other fields
}

type Metadata struct {
    // Name to identify this Zarf package.
	Name string `json:"name" jsonschema:"pattern=^[a-z0-9][a-z0-9\\-]*$"`
	// Additional information about this package.
	Description string `json:"description,omitempty"`

    //... other fields
}

// other structs

This way, it'll be easier to look at the whole API surface and better understand it's construct at the high level. This will help better understand which parts are duplicates, which should be reconsidered and how (see my earlier comment about Variant+Component). If we focus only on bits and pieces, we won't be able to look at the API holistically, thus we'll continue keeping the same problems we already have.

[soltysh]

Another example, I've noticed when looking at the zarf.yaml below was the repetition of this section:

        shell:
          darwin: sh
          linux: sh
          windows: powershell

does it really have to be repeated for every single action? There are 12 (!) instances of that section in that file. I'd personally not be super happy being forced to even copy&paste something like that over and over and over again, rather than defining that once. I might be wrong, and there might be good reasons why the shell needs to be defined for every action, but maybe we should have a global setting, and only then customization per each action?

[soltysh]

Those are the things, I'd like to be able to look at from a high vantage point of view, but for that we need that full structure of the API, if we want to make it right.

[AustinAbro321]

While this takes up a lot of space the in example yaml I don't see this as an issue since there isn't a real use case where you would copy and paste commands across all the different timings where it's possible to run commands. In the actual schema we re-use the objects. Users do have the ability to set defaults for a component already as well.

// ZarfComponentActions are ActionSets that map to different zarf package operations.
type ZarfComponentActions struct {
	// Actions to run during package creation.
	OnCreate ZarfComponentActionSet `json:"onCreate,omitempty"`
	// Actions to run during package deployment.
	OnDeploy ZarfComponentActionSet `json:"onDeploy,omitempty"`
	// Actions to run during package removal.
	OnRemove ZarfComponentActionSet `json:"onRemove,omitempty"`
}

// ZarfComponentActionSet is a set of actions to run during a zarf package operation.
type ZarfComponentActionSet struct {
	// Default configuration for all actions in this set.
	Defaults ZarfComponentActionDefaults `json:"defaults,omitempty"`
	// Actions to run at the start of an operation.
	Before []ZarfComponentAction `json:"before,omitempty"`
	// Actions to run at the end of an operation.
	After []ZarfComponentAction `json:"after,omitempty"`
	// Actions to run if all operations fail.
	OnFailure []ZarfComponentAction `json:"onFailure,omitempty"`
}

// ZarfComponentActionDefaults sets the default configs for child actions.
type ZarfComponentActionDefaults struct {
	// Hide the output of commands during execution (default false).
	Mute bool `json:"mute,omitempty"`
	// Default timeout for commands (default no timeout).
	Timeout *metav1.Duration `json:"timeout,omitempty"`
	// Retry commands given number of times if they fail (default 0).
	Retries int `json:"retries,omitempty"`
	// Working directory for commands (default CWD).
	Dir string `json:"dir,omitempty"`
	// Additional environment variables for commands.
	Env []string `json:"env,omitempty"`
	// (cmd only) Indicates a preference for a shell for the provided cmd to be executed in on supported operating systems.
	Shell Shell `json:"shell,omitempty"`
}

// ZarfComponentAction represents a single action to run during a zarf package operation.
type ZarfComponentAction struct {
	// Hide the output of the command during package deployment (default false).
	Mute *bool `json:"mute,omitempty"`
	// Timeout for the command (default to 0, no timeout for cmd actions and 5 minutes for wait actions).
	Timeout *metav1.Duration `json:"timeout,omitempty"`
	// Retry the command if it fails up to given number of times (default 0).
	Retries int `json:"retries,omitempty"`
	// The working directory to run the command in (default is CWD).
	Dir *string `json:"dir,omitempty"`
	// Additional environment variables to set for the command.
	Env []string `json:"env,omitempty"`
	// The command to run. Must specify either cmd or wait for the action to do anything.
	Cmd string `json:"cmd,omitempty"`
	// (cmd only) Indicates a preference for a shell for the provided cmd to be executed in on supported operating systems.
	Shell *Shell `json:"shell,omitempty"`
	// (onDeploy/cmd only) An array of variables to update with the output of the command. These variables will be available to all remaining actions and components in the package.
	SetVariables []Variable `json:"setVariables,omitempty"`
	// An array of values to set with the output of the command.
	SetValues []SetValue `json:"setValues,omitempty"`
	// Description of the action to be displayed during package execution instead of the command.
	Description string `json:"description,omitempty"`
	// Wait for a condition to be met before continuing. Must specify either cmd or wait for the action. See the 'zarf tools wait-for' command for more info.
	Wait *ZarfComponentActionWait `json:"wait,omitempty"`
	// Disable go-template processing on the cmd field.
	Template *bool `json:"template,omitempty"`
}

[AustinAbro321]

Added package.go and componentConfig.go which have the entire respective objects

[soltysh]

IIUC .version is used for version specification, that's what the text above states, but I'm missing that field here.

[AustinAbro321]

Whoops that was an old comment, I updated that comment. For Git repositories, rather than having a version field, we'll allow users to go to a specific branch, tag, or commit with Git URL syntax