MGMT-23769: Provide up front validation and handling of template prov… - #414
Conversation
|
@DakCrowder: This pull request references MGMT-23769 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@DakCrowder: This pull request references MGMT-23769 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPublic and private protos add Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/servers/private_compute_instances_server.go (1)
200-205:⚠️ Potential issue | 🟠 MajorApply spec_defaults when template changes during Update to match Create behavior.
The
Updatemethod validates the new template but does not apply itsspec_defaults, unlike theCreatemethod which explicitly callsApplySpecDefaults. When a user updatesspec.templateto a different template, the new template's defaults are silently ignored. SinceApplySpecDefaultsonly fills unset fields (preserving any user-provided values), applying defaults on template change would be safe and consistent with Create semantics. Without this, updates to a template could leave required fields missing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/servers/private_compute_instances_server.go` around lines 200 - 205, In Update, when detecting template changes via hasMaskPrefix(mask, "spec.template", "spec.template_parameters") and after calling s.fetchAndValidateTemplate(ctx, request.GetObject()), call ApplySpecDefaults on the incoming object (the same object passed to fetchAndValidateTemplate / request.GetObject()) to populate any unset fields from the new template; ensure you handle and return any error from ApplySpecDefaults consistently (like the existing error path) so Update matches Create's behavior of applying spec defaults when the template changes.
🧹 Nitpick comments (2)
internal/utils/spec_defaults.go (1)
28-30: Consider documenting the source of valid run strategies.The comment mentions these values match the Kubernetes ComputeInstance CRD. Consider adding a reference to where these values are defined in the CRD or noting that this list must be kept in sync with the CRD.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/utils/spec_defaults.go` around lines 28 - 30, Update the comment for validRunStrategies to reference the authoritative source and maintenance note: state exactly which Kubernetes ComputeInstance CRD (e.g., its API group/version or repo/manifest) defines the allowed values and add a concise "must be kept in sync" note so future maintainers know to update validRunStrategies when the CRD changes; mention the symbol validRunStrategies and the Kubernetes ComputeInstance CRD in the comment to make the linkage explicit.internal/servers/private_compute_instances_server_test.go (1)
702-706: Inconsistent DAO builder configuration.These inline template DAOs include
SetAttributionLogic(attribution)while thecreateTemplatehelper (Line 247-251) does not set attribution logic. This inconsistency could mask issues or cause test failures if attribution logic behavior changes.Consider aligning the DAO construction to be consistent, either always including or always omitting
SetAttributionLogic.Also applies to: 740-744, 780-784
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/servers/private_compute_instances_server_test.go` around lines 702 - 706, The test DAOs are built inconsistently: some inline NewGenericDAO[*privatev1.ComputeInstanceTemplate]() chains include SetAttributionLogic(attribution) while the createTemplate helper does not; update the createTemplate helper to include SetAttributionLogic(attribution) so all template DAOs use the same attribution logic (or alternatively remove SetAttributionLogic from the inline builders to match createTemplate) — modify the createTemplate helper (and mirror changes to any other helper used for templates) or the inline NewGenericDAO[...]() chains (the locations using SetAttributionLogic(attribution) in the ComputeInstanceTemplate DAO builders) to make the configuration consistent across tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/servers/private_compute_instances_server.go`:
- Around line 200-205: In Update, when detecting template changes via
hasMaskPrefix(mask, "spec.template", "spec.template_parameters") and after
calling s.fetchAndValidateTemplate(ctx, request.GetObject()), call
ApplySpecDefaults on the incoming object (the same object passed to
fetchAndValidateTemplate / request.GetObject()) to populate any unset fields
from the new template; ensure you handle and return any error from
ApplySpecDefaults consistently (like the existing error path) so Update matches
Create's behavior of applying spec defaults when the template changes.
---
Nitpick comments:
In `@internal/servers/private_compute_instances_server_test.go`:
- Around line 702-706: The test DAOs are built inconsistently: some inline
NewGenericDAO[*privatev1.ComputeInstanceTemplate]() chains include
SetAttributionLogic(attribution) while the createTemplate helper does not;
update the createTemplate helper to include SetAttributionLogic(attribution) so
all template DAOs use the same attribution logic (or alternatively remove
SetAttributionLogic from the inline builders to match createTemplate) — modify
the createTemplate helper (and mirror changes to any other helper used for
templates) or the inline NewGenericDAO[...]() chains (the locations using
SetAttributionLogic(attribution) in the ComputeInstanceTemplate DAO builders) to
make the configuration consistent across tests.
In `@internal/utils/spec_defaults.go`:
- Around line 28-30: Update the comment for validRunStrategies to reference the
authoritative source and maintenance note: state exactly which Kubernetes
ComputeInstance CRD (e.g., its API group/version or repo/manifest) defines the
allowed values and add a concise "must be kept in sync" note so future
maintainers know to update validRunStrategies when the CRD changes; mention the
symbol validRunStrategies and the Kubernetes ComputeInstance CRD in the comment
to make the linkage explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb53957c-425e-4f59-a0c3-e7ba1169c0a0
⛔ Files ignored due to path filters (4)
internal/api/osac/private/v1/compute_instance_template_type.pb.gois excluded by!**/*.pb.gointernal/api/osac/private/v1/compute_instance_template_type_protoopaque.pb.gois excluded by!**/*.pb.gointernal/api/osac/public/v1/compute_instance_template_type.pb.gois excluded by!**/*.pb.gointernal/api/osac/public/v1/compute_instance_template_type_protoopaque.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (8)
internal/servers/compute_instances_server_test.gointernal/servers/private_compute_instances_server.gointernal/servers/private_compute_instances_server_test.gointernal/testing/database.gointernal/utils/spec_defaults.gointernal/utils/spec_defaults_test.goproto/private/osac/private/v1/compute_instance_template_type.protoproto/public/osac/public/v1/compute_instance_template_type.proto
d166ccf to
761ae48
Compare
|
@DakCrowder: This pull request references MGMT-23769 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
8de0540 to
36d3083
Compare
|
@DakCrowder: This pull request references MGMT-23769 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/servers/private_compute_instances_server.go`:
- Around line 198-214: Currently the code fetches/validates the template from
the sparse request before merging the current object, causing validation to use
request.template_parameters instead of the effective post-update spec; change
the order so you first load the current instance (fetchCurrentInstance) and call
mergeSpecFromMask(current.GetSpec(), request.GetSpec(), mask) to produce
mergedSpec, then derive/resolve the template from that mergedSpec (replace
fetchAndValidateTemplate(ctx, request.GetObject()) with a fetch/validate that
takes mergedSpec or its template reference), and finally call
validateSpecWithDefaults(mergedSpec, template); apply the same reorder to the
analogous block later in the file that mirrors this logic (the code using
hasMaskPrefix, fetchAndValidateTemplate, mergeSpecFromMask, and
validateSpecWithDefaults).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a280fcf-ae30-44f9-a2da-5e49cd7a7f4b
⛔ Files ignored due to path filters (4)
internal/api/osac/private/v1/compute_instance_template_type.pb.gois excluded by!**/*.pb.gointernal/api/osac/private/v1/compute_instance_template_type_protoopaque.pb.gois excluded by!**/*.pb.gointernal/api/osac/public/v1/compute_instance_template_type.pb.gois excluded by!**/*.pb.gointernal/api/osac/public/v1/compute_instance_template_type_protoopaque.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (8)
internal/servers/compute_instances_server_test.gointernal/servers/private_compute_instances_server.gointernal/servers/private_compute_instances_server_test.gointernal/testing/database.gointernal/utils/spec_defaults.gointernal/utils/spec_defaults_test.goproto/private/osac/private/v1/compute_instance_template_type.protoproto/public/osac/public/v1/compute_instance_template_type.proto
🚧 Files skipped from review as they are similar to previous changes (4)
- internal/testing/database.go
- internal/servers/compute_instances_server_test.go
- internal/utils/spec_defaults_test.go
- proto/private/osac/private/v1/compute_instance_template_type.proto
00b2f5d to
1185cb2
Compare
|
@DakCrowder: This pull request references MGMT-23769 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/servers/private_compute_instances_server.go (1)
264-275:⚠️ Potential issue | 🟠 MajorPreserve
InvalidArgumentfor missing templates.Lines 345-353 and 383-391 already translate
*dao.ErrNotFoundfromGenericDAO.Get().Do(ctx)intoInvalidArgument, but this helper maps every lookup error toInternal. A missing template will therefore surface as a 500 instead of a bad user reference. Handle*dao.ErrNotFoundhere before falling back to the generic internal error.🐛 Proposed fix
getTemplateResponse, err := s.templatesDao.Get(). SetId(templateID). Do(ctx) if err != nil { + var notFoundErr *dao.ErrNotFound + if errors.As(err, ¬FoundErr) { + return nil, grpcstatus.Errorf( + grpccodes.InvalidArgument, + "template '%s' does not exist", + templateID, + ) + } s.logger.ErrorContext( ctx, "Template retrieval failed", slog.String("template_id", templateID), slog.Any("error", err), ) return nil, grpcstatus.Errorf( grpccodes.Internal, "failed to retrieve template '%s'", templateID, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/servers/private_compute_instances_server.go` around lines 264 - 275, The current error handling in the template retrieval block maps all errors to Internal; instead detect whether err is a *dao.ErrNotFound and, if so, log and return grpcstatus.Errorf(grpccodes.InvalidArgument, "template '%s' not found", templateID) (preserving the s.logger.ErrorContext call and its fields), otherwise keep the existing Internal log+grpcstatus.Errorf behavior; reference dao.ErrNotFound, s.logger.ErrorContext, templateID, grpcstatus.Errorf and grpccodes.Internal/InvalidArgument and add an import for the dao package if not already present.
♻️ Duplicate comments (1)
internal/servers/private_compute_instances_server.go (1)
198-205:⚠️ Potential issue | 🟠 MajorValidate the effective post-update spec before calling
generic.Update.A sparse patch that only touches
spec.template_parametersreaches Line 233 with nospec.template, so this path rejects a valid update with"template ID is mandatory". It also still skipsvalidateSpecWithDefaults, so aspec.templatechange can swap to a template with nospec_defaultsand persist an instance that no longer has all required spec fields. Fetch the current object, merge the masked spec first, then resolve the effective template and validate that merged spec once.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/servers/private_compute_instances_server.go` around lines 198 - 205, The current flow validates only when mask includes spec.template or spec.template_parameters and then calls s.generic.Update, which can persist an invalid effective spec; instead, fetch the current object (via existing fetch method used elsewhere), merge the incoming masked spec into the current object's spec before calling fetchAndValidateTemplate/validateSpecWithDefaults to resolve the effective template and apply defaults, ensure the merged/validated spec passes validation, and only then call s.generic.Update with the original request (or with the merged spec applied) so updates that only touch template_parameters or remove template-required defaults are correctly validated; use hasMaskPrefix, fetchAndValidateTemplate, validateSpecWithDefaults and s.generic.Update as the reference points when implementing this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/utils/spec_defaults.go`:
- Around line 28-30: validRunStrategies currently only contains "Always" and
"Halted" but the code path that validates run strategies (referenced around the
validation that errors for invalid run strategies) is intended to match
KubeVirt's VirtualMachine CRD which actually accepts "Always", "RerunOnFailure",
"Once", "Manual", and "Halted"; update validRunStrategies to include all five
values ("Always","RerunOnFailure","Once","Manual","Halted") so upstream inputs
are accepted, and if you intentionally want to restrict values instead, change
the top comment above validRunStrategies and the validation error message that
references this check to state this is a product-specific restriction rather
than a CRD match (reference symbols: validRunStrategies and the run-strategy
validation code that produces the error).
---
Outside diff comments:
In `@internal/servers/private_compute_instances_server.go`:
- Around line 264-275: The current error handling in the template retrieval
block maps all errors to Internal; instead detect whether err is a
*dao.ErrNotFound and, if so, log and return
grpcstatus.Errorf(grpccodes.InvalidArgument, "template '%s' not found",
templateID) (preserving the s.logger.ErrorContext call and its fields),
otherwise keep the existing Internal log+grpcstatus.Errorf behavior; reference
dao.ErrNotFound, s.logger.ErrorContext, templateID, grpcstatus.Errorf and
grpccodes.Internal/InvalidArgument and add an import for the dao package if not
already present.
---
Duplicate comments:
In `@internal/servers/private_compute_instances_server.go`:
- Around line 198-205: The current flow validates only when mask includes
spec.template or spec.template_parameters and then calls s.generic.Update, which
can persist an invalid effective spec; instead, fetch the current object (via
existing fetch method used elsewhere), merge the incoming masked spec into the
current object's spec before calling
fetchAndValidateTemplate/validateSpecWithDefaults to resolve the effective
template and apply defaults, ensure the merged/validated spec passes validation,
and only then call s.generic.Update with the original request (or with the
merged spec applied) so updates that only touch template_parameters or remove
template-required defaults are correctly validated; use hasMaskPrefix,
fetchAndValidateTemplate, validateSpecWithDefaults and s.generic.Update as the
reference points when implementing this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce896e17-0f4e-4fcc-98c4-f85dc72cc13f
⛔ Files ignored due to path filters (4)
internal/api/osac/private/v1/compute_instance_template_type.pb.gois excluded by!**/*.pb.gointernal/api/osac/private/v1/compute_instance_template_type_protoopaque.pb.gois excluded by!**/*.pb.gointernal/api/osac/public/v1/compute_instance_template_type.pb.gois excluded by!**/*.pb.gointernal/api/osac/public/v1/compute_instance_template_type_protoopaque.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (9)
internal/servers/compute_instances_server_test.gointernal/servers/private_compute_instances_server.gointernal/servers/private_compute_instances_server_test.gointernal/testing/database.gointernal/utils/spec_defaults.gointernal/utils/spec_defaults_test.goit/it_compute_subnet_test.goproto/private/osac/private/v1/compute_instance_template_type.protoproto/public/osac/public/v1/compute_instance_template_type.proto
✅ Files skipped from review due to trivial changes (3)
- internal/testing/database.go
- proto/public/osac/public/v1/compute_instance_template_type.proto
- internal/utils/spec_defaults_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/servers/compute_instances_server_test.go
- proto/private/osac/private/v1/compute_instance_template_type.proto
|
@DakCrowder: This pull request references MGMT-23769 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@DakCrowder: This pull request references MGMT-23769 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
| handle, err := sql.Open("pgx", url) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Eventually(handle.Ping, 10, 1).ShouldNot(HaveOccurred()) | ||
| Eventually(handle.Ping, 30, 1).ShouldNot(HaveOccurred()) |
There was a problem hiding this comment.
My local environment (on my poor little thinkpad) was consistently timing out here running tests, bumped this up due to hitting it.
|
@DakCrowder I see the tests at L650-675 assert that template defaults are intentionally not stored on the object ("the template is the source of truth at render time"). I understand the reasoning, but I'm wondering how this works with the current CRD and controller. When the controller reconciles, The ticket (MGMT-23769) has two expectations: (1) defaults populated automatically from the template, and (2) clear error when info is missing. This PR addresses (2) well. For (1), is the plan to also update the controller to merge template defaults in |
|
|
||
| // Create a compute instance without any spec fields — validation should pass | ||
| // because template defaults cover all required fields, but defaults should NOT | ||
| // be stored on the object (the template is the source of truth at render time): |
There was a problem hiding this comment.
Curious about the design intent here. The test asserts defaults are NOT stored, with the comment "the template is the source of truth at render time." Could you help me understand how the controller handles this case? Looking at addExplicitFields(), it only populates fields where Has*() is true, so the K8s CR would be created without these spec fields. Since the CRD requires them, would the CR creation fail?
There was a problem hiding this comment.
Yes good observation / ty for raising. I had been waffling on the right approach here but I was missing a piece of the puzzle - which is the vendored mass open cloud aap arg spec has additional fields defined as template_parameters so I was seeing things populated in different ways that i was expecting.
After some more thought, I think there is a simpler option here - inspired by this comment on the associated aap pr I wonder if the defaults should just be template parameters with some well-known name or flag to indicate they are spec fields, we use the info defined (e.g. required, default value etc.) from the template parameter field definition to perform the validation, and then save them as a part of that block (continues behavior as-is of the template parameters are saved).
There was a problem hiding this comment.
I guess one difference / potential issue there is the template parameters only support a "flat" structure in comparison to nested spec fields like the image / boot_disk.
There was a problem hiding this comment.
To follow up on this - the code is now taking a snapshot of template default values at creation time (which is how we handle template_parameters) and store them - ensuring the fields are populated before the CR creation runs in the reconciler.
a7566a3 to
af48068
Compare
b7e6e15 to
53e2048
Compare
| @@ -189,7 +195,7 @@ func (s *PrivateComputeInstancesServer) Update(ctx context.Context, | |||
| } | |||
| } | |||
| if hasMaskPrefix(mask, "spec.template", "spec.template_parameters") { | |||
There was a problem hiding this comment.
Changing spec.template is allowed on update, but only fetchAndValidateTemplate runs here (template parameter validation). applySpecDefaults is not called, so the new template's spec defaults won't be applied for any fields the user hasn't explicitly set. The old template's defaults remain on the CI.
Two options:
- Call
applySpecDefaultshere too, so the new template's defaults take effect on update. - Block template changes post-creation (make
spec.templateimmutable after create), if switching templates on a running CI isn't intended to be supported.
Either way, the current behavior (silently keeping the old template's defaults after a template change) seems like it could surprise users.
There was a problem hiding this comment.
I was punting on update until I could get more clarity BUT after looking at clusters there is a method validateTemplateImmutability that (as named) ensures the corresponding fields on clusters cannot be changed on objects. Additionally, the compute_instace_type.proto indicates the template and template parameters should be immutable. Given this I think matching behavior can be introduced here aligned with option 2 above. If we want to change this behavior later then we can take it on more holistically alongside clusters / how we view templates as a whole.
There was a problem hiding this comment.
Updated to add the same behavior and ensures immutability as the proto def indicates
…ided spec default values for computeinstances
…een user provided values and default spec values
…ances when updating
53e2048 to
314cb35
Compare
akshaynadkarni
left a comment
akshaynadkarni
left a comment
There was a problem hiding this comment.
Code changes LGTM.
Prior to merging, can you update the PR description? It will be good to include any test results etc. so it's clear what the new behavior will be. Thanks.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akshaynadkarni, DakCrowder The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/unhold |
3 participants
Adds validation and initialization of compute instances using fetched template spec default values.
The gist of this is we fetch the template (was already occurring), but the template we get back now includes spec defaults. We take the compute instance request body, populate fields on the request body from the spec defaults (such as cpu cores, memory etc.) and then validate the combined payload. The result is users can get additional feedback on fields they must define if the template doesn't define them, and we populate the initial values from the spec defaults if the user doesn't provide specific values.
For example, if we had a template that had NO defaults defined, the user can now see they need to define fields such as cores, memory etc. at creation time rather than having to debug why their VM won't spin up later on.
Additionally if users specify a template with the defaults defined those defaults will be used to populate the computinstance.
User values always have precedence - so in a case like the following the user passed cores and memory will override the template default, while the other values will be sourced from the template.
Additionally if a template does NOT define a value (e.g. lets say cores is NOT specified in the default spec values), the user then must provide that value.
Requires osac-project/osac-aap#246 to fetch the default fields from templates
Summary by CodeRabbit
New Features
Improvements
Tests