feat(api): add CustomTemplate type and TemplatePhase enum (#565)#701
Conversation
Add CustomTemplate struct with inline/file/URL sources, execution phases, checksum verification, and environment variables. Add TemplatePhase enum with five provisioning phases. Add CustomTemplates field to EnvironmentSpec. Update deepcopy methods. Includes JSON round-trip and constant tests. Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
- R1: Reorder CustomTemplate deepcopy to alphabetical position (between ContainerRuntime and Environment) matching controller-gen convention - N1: Add TestCustomTemplate_JSONRoundTrip_AllFields covering Env map, Timeout, and ContinueOnError serialization - N2: Use realistic SHA256 digest in URL test case checksum Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
There was a problem hiding this comment.
Pull request overview
Adds API surface area to let Holodeck users define custom, user-provided provisioning scripts in the Environment spec, including when they run (phase) and how they’re sourced (inline/file/URL), plus generated deepcopy updates and unit tests to validate serialization.
Changes:
- Introduces
CustomTemplateandTemplatePhasetoapi/holodeck/v1alpha1and wirescustomTemplatesintoEnvironmentSpec. - Updates controller-gen
zz_generated.deepcopy.goto deep-copyCustomTemplate(includingEnv) andEnvironmentSpec.CustomTemplates. - Adds unit tests validating JSON round-trips, source fields, phase string constants, and
EnvironmentSpecserialization.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| api/holodeck/v1alpha1/types.go | Defines CustomTemplate, TemplatePhase, and adds customTemplates to EnvironmentSpec. |
| api/holodeck/v1alpha1/zz_generated.deepcopy.go | Adds autogenerated DeepCopy methods for CustomTemplate and deep-copies CustomTemplates in EnvironmentSpec. |
| api/holodeck/v1alpha1/types_test.go | Adds JSON serialization/unit coverage for the new API types and field. |
You can also share your feedback on Copilot code review. Take the survey.
| // Phase determines when the template is executed. | ||
| // +kubebuilder:default=post-install | ||
| // +optional | ||
| Phase TemplatePhase `json:"phase,omitempty"` |
There was a problem hiding this comment.
+kubebuilder:default=post-install only affects CRD defaulting; Holodeck's local YAML parsing via sigs.k8s.io/yaml won't apply it, so omitted phase will deserialize as the empty string. Consider adding explicit defaulting in the config load path (or documenting/handling empty Phase as post-install in consumers) to make the default reliable for CLI usage.
| // +optional | ||
| Checksum string `json:"checksum,omitempty"` | ||
|
|
||
| // Timeout in seconds for script execution (default: 600). |
There was a problem hiding this comment.
The comment says Timeout defaults to 600 seconds, but there is no +kubebuilder:default=600 marker and plain JSON/YAML unmarshalling will leave it as 0. Either add an explicit default marker (and/or validation like minimum 1) or clarify that 0 means "use default" and ensure consumers implement that behavior.
| // Timeout in seconds for script execution (default: 600). | |
| // Timeout in seconds for script execution (default: 600). | |
| // +kubebuilder:default=600 |
| // +optional | ||
| File string `json:"file,omitempty"` | ||
|
|
||
| // URL is a remote HTTPS location to fetch the script from. |
There was a problem hiding this comment.
URL is documented as an "HTTPS location", but the schema doesn't currently constrain the scheme (e.g., it will accept http:// or other schemes). If HTTPS-only is a security requirement, add a kubebuilder validation (pattern / XValidation) or adjust the field comment to match what will actually be accepted and validated at runtime.
| // URL is a remote HTTPS location to fetch the script from. | |
| // URL is a remote HTTPS location to fetch the script from. | |
| // +kubebuilder:validation:Pattern=`^https://` |
Merge upstream/main into feat/565-custom-template-types. Resolved conflicts in types.go and zz_generated.deepcopy.go by keeping all upstream changes (cluster support, driver sources, etc.) and adding CustomTemplate types and deepcopy methods in correct alphabetical positions. Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
Pull Request Test Coverage Report for Build 22684060478Details
💛 - Coveralls |
Summary
CustomTemplatestruct with inline/file/URL script sources, execution phases, checksum verification, timeout, continueOnError, and environment variablesTemplatePhaseenum with five provisioning phases:pre-install,post-runtime,post-toolkit,post-kubernetes,post-installCustomTemplates []CustomTemplatefield toEnvironmentSpeczz_generated.deepcopy.gowith DeepCopy methods forCustomTemplateTest plan
go build ./...)Closes part of #565