add support for reserved default object param #164
Conversation
Yongxuanzhang
force-pushed
the
param-to-remote-patch
branch
4 times, most recently
from
1f81f21
to
e2e1633
Compare
| taskSpec = "taskSpec" | ||
| pipelineRef = "pipelineRef" | ||
| pipelineSpec = "pipelineSpec" | ||
| ReservedParamName = "TBD" |
There was a problem hiding this comment.
will add changes to v1beta1 soon
Yongxuanzhang
force-pushed
the
param-to-remote-patch
branch
from
e2e1633
to
8d16c7b
Compare
| @@ -132,6 +132,7 @@ func ValidateUsageOfDeclaredParameters(ctx context.Context, steps []Step, params | |||
| var errs *apis.FieldError | |||
| _, _, objectParams := params.SortByType() | |||
| allParameterNames := sets.NewString(params.GetNames()...) | |||
| allParameterNames.Insert(ReservedParamName) | |||
There was a problem hiding this comment.
May be here we should check that the ReservedParamName is not already in the set. If it is then a user also declared it and it should throw an error since it is reserved?
There was a problem hiding this comment.
this check is not done from our side, there will be additional validation out of tekton and we're sure that users cannot declare the param with reserved name. :D
There was a problem hiding this comment.
(and we cannot do it from out side for some reason...
| @@ -395,6 +396,7 @@ func (ps *PipelineSpec) validatePipelineParameterUsage(ctx context.Context) (err | |||
| // validatePipelineTaskParameterUsage validates that parameters referenced in the Pipeline Tasks are declared by the Pipeline | |||
| func validatePipelineTaskParameterUsage(tasks []PipelineTask, params ParamSpecs) (errs *apis.FieldError) { | |||
| allParamNames := sets.NewString(params.GetNames()...) | |||
| allParamNames.Insert(ReservedParamName) | |||
There was a problem hiding this comment.
May be here we should check that the ReservedParamName is not already in the set. If it is then a user also declared it and it should throw an error since it is reserved?
| @@ -671,6 +671,20 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel | |||
| } | |||
|
|
|||
| if pipelineRunFacts.State.IsBeforeFirstTaskRun() { | |||
| // check if the object's key names are referenced correctly i.e. param.objectParam.key1 | |||
There was a problem hiding this comment.
working on adding unit tests for this, same for taskrun. e2e tests have been added to cover this code
Yongxuanzhang
force-pushed
the
param-to-remote-patch
branch
from
8d16c7b
to
0c5546b
Compare
Yongxuanzhang
force-pushed
the
param-to-remote-patch
branch
5 times, most recently
from
0d050fc
to
d4fe76d
Compare
This commit adds support for reserved default object param.Users can reference to default object param values without needing to declaring them in remote Task&Pipeline. Validation err will be returned if referencing to non-existent object key. Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com
Yongxuanzhang
force-pushed
the
param-to-remote-patch
branch
from
d4fe76d
to
d62e1e6
Compare
Changes
This commit adds support for reserved default object param.Users can reference to default object param values without needing to declaring them in remote Task&Pipeline. Validation err will be returned if referencing to non-existent object key.
This is the PR to run all tests: tektoncd#7484
Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes