ZEP-0051: v1beta1 schema#52
Conversation
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
soltysh
left a comment
There was a problem hiding this comment.
Have a look at a rough sketch where I've implemented some of the comments I've mentioned above, but also a few others: https://gist.github.com/soltysh/eaab0ac6a0113af60c3ce1013b85cfc7
| // The kind of Zarf package. | ||
| Kind PackageKind `json:"kind" jsonschema:"enum=ZarfInitConfig,enum=ZarfPackageConfig,default=ZarfPackageConfig"` | ||
| // Package metadata. | ||
| Metadata Metadata `json:"metadata,omitempty"` |
There was a problem hiding this comment.
Let's merge Metadata and ComponentMetadata together with the following fields: Name, Description, Version and Annotations.
Additionally, let's introduce a single Kind enum with both values PackageConfig and ComponentConfig (I've explicitly dropped the Zarf* from both).
Then each of the top-level structs will look like the following:
type Package struct {
APIVersion string `json:"apiVersion" jsonschema:"enum=zarf.dev/v1beta1"`
Kind Kind `json:"kind" jsonschema:"enum=PackageConfig,default=PackageConfig"`
Metadata Metadata `json:"metadata,omitempty"`
...
}
type ComponentConfig struct {
APIVersion string `json:"apiVersion" jsonschema:"enum=zarf.dev/v1beta1"`
Kind Kind `json:"kind" jsonschema:"enum=ComponentConfig,default=ComponentConfig"`
Metadata Metadata `json:"metadata,omitempty"`
...
}
type Metadata struct {
Name string `json:"name" jsonschema:"pattern=^[a-z0-9][a-z0-9\\-]*$"`
Description string `json:"description,omitempty"`
Version string `json:"version,omitempty"`
Annotations map[string]string `json:"annotations,omitempty"`
}I was also considering sharing APIVersion and Kind, but then validation will be hard, and it seems like an overkill.
There was a problem hiding this comment.
I don't think I like moving architecture, uncompressed, and allowNameSpaceOverrides into another field like packageSpec. Thinking from the experience of the package author, it may feel arbitrary to have those fields in a separate struct rather than setting them in metadata.
| // Annotations are key-value pairs that can be used to store metadata about the package. | ||
| Annotations map[string]string `json:"annotations,omitempty"` | ||
| // Whether to allow namespace overrides for this package. | ||
| AllowNamespaceOverride *bool `json:"allowNamespaceOverride,omitempty"` |
There was a problem hiding this comment.
- Move Uncompressed, Architecture and AllowNamespaceOverride to package definition.
- Why do we need Architecture here, if we already specify that on a per-component target? It seems confusing. If this is not binding and I believe it's not, but more like a suggestion, I'd push to either placing that in annotations or documentation.
There was a problem hiding this comment.
- I'd rather keep them in metadata, as I think it provides a more clear UX, to have all these fields in the same place
- Architecture is binding as it decides the platforms of the images are pulled and which components are built. Note that architecture can be set from
.metadata.architecturebut can also be set from the command line (e.g.zarf package create -a arm64). So.component.target.architectureis dependent on.metdata.architecture, whencomponent.target.architectureis set, the component is only built if that architecture matches.metadata.architecture
| // Values imports Zarf values files for templating and overriding Helm values. | ||
| Values Values `json:"values,omitempty"` | ||
| // Documentation files included in the package. | ||
| Documentation map[string]string `json:"documentation,omitempty"` |
There was a problem hiding this comment.
Do we really need Annotations and Documentation? From a type perspective they don't differ at all, so what's the difference?
There was a problem hiding this comment.
There's also Description, which is part of Metadata, so I believe there's plenty of places to keep those values.
There was a problem hiding this comment.
documentation is a way to store files in the package that document how the package is used. Whereas annotations are strings. View an example of documentation here https://github.com/zarf-dev/zarf/blob/main/examples/dos-games/zarf.yaml
| // Generic string set by a package author to track the package version. | ||
| Version string `json:"version,omitempty"` | ||
| // Disable compression of this package. | ||
| Uncompressed bool `json:"uncompressed,omitempty"` |
There was a problem hiding this comment.
From json marshalling pov there's no difference between empty value and false value. Those are considered identical. This means that whatever the user sets explicitly here it might get lost. Iow. both: Uncompressed=false and unset will marshal into empty field b/c of omitempty. Is that the intention here and with all of the rest of the bool fields? Or do we want to be explicit about that differentiation?
There was a problem hiding this comment.
So with the uncompressed field, it's fine because uncompressed=false is the default, so whether or not a user set uncompressed: false or not the file will be compressed. With fields where the default is not false, we use bool pointers and function on the package struct that gives the default. For instance
func (zc ZarfChart) ShouldRunSchemaValidation() bool {
if zc.SchemaValidation != nil {
return *zc.SchemaValidation
}
return true
}Though this is not 100% consistent since some fields that have a default of false also use bool pointers. Do you think we should use bool pointers everywhere, or only use them when boolean fields default to true?
| // - "auto": use SSA for fresh installs; for upgrades, match whichever strategy | ||
| // was used when the chart was first installed | ||
| // Defaults to "auto" when omitted. | ||
| ServerSideApply string `json:"serverSideApply,omitempty" jsonschema:"enum=true,enum=false,enum=auto"` |
| // Local folder or file path or remote URL to pull into the package. | ||
| Source string `json:"source"` | ||
| // Optional SHA256 checksum of the file. | ||
| Shasum string `json:"shasum,omitempty"` |
There was a problem hiding this comment.
Hash or Checksum maybe to make this less tide to specific implementation?
There was a problem hiding this comment.
In that case would we require giving the specific implementation like sha256:<shasum> . I do prefer the ergonomics of not having to specify the algorithm. Especially since we have a command zarf dev sha256sum, but I see the value in not tying ourselves to a specific implementation.
I'm thinking we change the field to checksum, and if a user doesn't write an algorithm (e.g. checksum: 8e74dbc7ee40d8....) we treat it as a sha256 sum. Or they can write checksum: sha256:8e74dbc7ee40d8.... Any future algorithms we implement would require giving the algorithm upfront.
| ExtractPath string `json:"extractPath,omitempty"` | ||
| // Template enables go-template processing on this file during deploy. | ||
| Template *bool `json:"template,omitempty"` | ||
| } |
There was a problem hiding this comment.
Can you walk me through the various fields here? I feel like I'm a bit confused when source is directory or remote URL and how this plays with the following:
- Symlinks, why this is a list where others are just a single fields
- ExtractPath, what is that? and partially also how this relates to not being a list?
- Template, is it needed here?
There was a problem hiding this comment.
If, we're handling a single file it all makes sense to me, but once we start talking about input being a directory, then the mixture of lists (Symlinks) vs single value fields (Executable, ExtractPath) is confusing.
There was a problem hiding this comment.
- Symlinks is a list of symlinks that will be created on the host machine during
zarf package deploy. symlinks works for both files and directories - extract path only works if the file is an archive (tar/zip/any). If it is then Zarf will look inside the archive and only extract what is at the extract path to the target destination.
- template decides whether or not we go template the file.
To me a list of symlinks makes sense since a single file or directory can be symlinked to multiple times. Executable makes either the file executable or every file in the directory executable. There is some weirdness in that extractPath only works on archived files.
| // ComponentActionDefaults sets the default configs for child actions. | ||
| type ComponentActionDefaults struct { | ||
| // Hide the output of commands during execution (default false). | ||
| Mute bool `json:"mute,omitempty"` |
There was a problem hiding this comment.
I like mute better, since I think quiet could be misconstrued as less output whereas mute makes it clear that there is zero output
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Uh oh!
There was an error while loading. Please reload this page.