Validate PipelineTask name as Task names 📛 #2099
Conversation
googlebot
added
the
cla: yes
tekton-robot
added
the
size/M
|
The following is the coverage report on pkg/.
|
| return &apis.FieldError{ | ||
| Message: fmt.Sprintf("invalid value %q", t.Name), | ||
| Paths: []string{fmt.Sprintf("spec.tasks[%d].name", i)}, | ||
| Details: "Pipeline Task step name must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names", |
There was a problem hiding this comment.
Do we want the error message to be consistent with Task?
The Task "FooTask" is invalid: metadata.name: Invalid value: "FooTask": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
Remove step after Pipeline Task:
| Details: "Pipeline Task step name must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names", | |
| Details: "Pipeline Task name must be a valid DNS Label. For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names", |
There was a problem hiding this comment.
Seeing there was another push here, but I am not seeing an update to the error message.
My thought is to make it so the error message here is consistent with what is shown for a Task that a user tries to create the violates naming conventions.
vdemeester
force-pushed
the
2095-pipelinetask-validation
branch
from
ad1924c
to
b93fd4e
Compare
|
The following is the coverage report on pkg/.
|
|
@vdemeester Can this comment below be addressed before this goes in?
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbwsg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
vdemeester
force-pushed
the
2095-pipelinetask-validation
branch
from
b93fd4e
to
ff3a6ce
Compare
|
The following is the coverage report on pkg/.
|
As reported, a `Pipeline` task name (`Pipelinetask`) can be created that will violate `TaskRun` naming rules, making it fail at runtime. This changes this by validation PipelineTask name the same way we validation `TaskRun` names, to make sure we won't fail for this at runtime. Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
vdemeester
force-pushed
the
2095-pipelinetask-validation
branch
from
ff3a6ce
to
64b139f
Compare
|
The following is the coverage report on pkg/.
|
|
/lgtm |
Changes
As reported, a
Pipelinetask name (Pipelinetask) can be createdthat will violate
TaskRunnaming rules, making it fail at runtime.This changes this by validation PipelineTask name the same way we
validation
TaskRunnames, to make sure we won't fail for this atruntime.
Closes #2095
Signed-off-by: Vincent Demeester vdemeest@redhat.com
/cc @bobcatfish @afrittoli @danielhelfand
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmddir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes