Verifier for tasks and plans #1224
Conversation
Signed-off-by: Ken Sipe <kensipe@gmail.com>
kensipe
requested review from
alenkacz,
gerred,
nfnt and
zen-dog
as code owners
|
needs some tests but otherwise good to go |
pkg/kudoctl/cmd/verify/verify.go
Outdated
| @@ -12,6 +12,7 @@ import ( | |||
| var verifiers = []verifier.PackageVerifier{ | |||
| DuplicateVerifier{}, | |||
| InvalidCharVerifier{";,"}, | |||
| PlanVerifier{}, | |||
There was a problem hiding this comment.
This feels very similar to ReferenceVerifier, shouldn't these two be one thing? Or at least be structured in the same way? This one lives directly in this file while the other is in templates package. Also this is also checking references, so if we keep them separate we should probably think about better names - like TasksVerifier and TemplateVerifier?
There was a problem hiding this comment.
yeah... the Param and Reference verifiers are in separate packages to completely encapsulate templating (which is something we don't control). any changes (however unlikely) are encapsulated from this package of verifying.
Plan verifying is wholly managed by our code. It has completely different concerns. All other project verifiers are here so I didn't originally see any reason to move it.
re: "this feels very similar to ReferenceVerifier". That seems confusing to me... the only thing that makes sense to me is naming... I'll look at that. However "ReferenceVerifier" is "template.ReferenceVerifier" which is the verifier to check template references. The "PlanVerifier" is written to verify the operator plans. however it really is a Task Reference Verifier.
While I'm inclined to leave the verifier here as noted above... perhaps for consistency moving it to "task.ReferenceVerifier" would make sense. It isn't necessary for encapsulation IMO but it doesn't hurt... and provides readability.
There was a problem hiding this comment.
moved to task.ReferenceVerifier
Signed-off-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Ken Sipe <kensipe@gmail.com> Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Signed-off-by: Ken Sipe kensipe@gmail.com
What this PR does / why we need it:
Realized with lots of work on operators and skeleton generators that we could use verification on plan tasks and defined tasks since they are loosely coupled by name.
This PR provides a new "PlanVerifier" which provides warnings for tasks that are defined but are not used in a plan and an error for tasks in a plan that are not defined.
Fixes #