Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert removal of PipelineResources related fields #6436

Merged
merged 1 commit into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,349 changes: 1,255 additions & 94 deletions docs/pipeline-api.md

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions hack/update-codegen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ GOFLAGS="-mod=vendor"
# --output-base because this script should also be able to run inside the vendor dir of
# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
# instead of the $GOPATH directly. For normal projects this can be dropped.

# This generates deepcopy,client,informer and lister for the resource package (v1alpha1)
# This is separate from the pipeline package as resource are staying in v1alpha1 and they
# need to be separated (at least in terms of go package) from the pipeline's packages to
# not having dependency cycle.
bash ${REPO_ROOT_DIR}/hack/generate-groups.sh "deepcopy,client,informer,lister" \
github.com/tektoncd/pipeline/pkg/client/resource github.com/tektoncd/pipeline/pkg/apis \
"resource:v1alpha1" \
--go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt

# This generates deepcopy,client,informer and lister for the pipeline package (v1alpha1, v1beta1, and v1)
bash ${REPO_ROOT_DIR}/hack/generate-groups.sh "deepcopy,client,informer,lister" \
github.com/tektoncd/pipeline/pkg/client github.com/tektoncd/pipeline/pkg/apis \
Expand Down Expand Up @@ -70,6 +80,14 @@ ${PREFIX}/deepcopy-gen \
-i github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1

# Knative Injection

# This generates the knative injection packages for the resource package (v1alpha1).
# This is separate from the pipeline package for the same reason as client and all (see above).
bash ${REPO_ROOT_DIR}/hack/generate-knative.sh "injection" \
github.com/tektoncd/pipeline/pkg/client/resource github.com/tektoncd/pipeline/pkg/apis \
"resource:v1alpha1" \
--go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt

# This generates the knative inject packages for the pipeline package (v1alpha1, v1beta1, v1).
bash ${REPO_ROOT_DIR}/hack/generate-knative.sh "injection" \
github.com/tektoncd/pipeline/pkg/client github.com/tektoncd/pipeline/pkg/apis \
Expand Down
683 changes: 677 additions & 6 deletions pkg/apis/pipeline/v1beta1/openapi_generated.go

Large diffs are not rendered by default.

11 changes: 11 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ type PipelineSpec struct {
// used to populate a UI.
// +optional
Description string `json:"description,omitempty"`
// Deprecated: Unused, preserved only for backwards compatibility
// +listType=atomic
Resources []PipelineDeclaredResource `json:"resources,omitempty"`
// Tasks declares the graph of Tasks that execute when this Pipeline is run.
// +listType=atomic
Tasks []PipelineTask `json:"tasks,omitempty"`
Expand Down Expand Up @@ -201,6 +204,10 @@ type PipelineTask struct {
// +listType=atomic
RunAfter []string `json:"runAfter,omitempty"`

// Deprecated: Unused, preserved only for backwards compatibility
// +optional
Resources *PipelineTaskResources `json:"resources,omitempty"`

// Parameters declares parameters passed to this task.
// +optional
// +listType=atomic
Expand Down Expand Up @@ -451,6 +458,10 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) {

errs = errs.Also(pt.validateEmbeddedOrType())

if pt.Resources != nil {
errs = errs.Also(apis.ErrDisallowedFields("resources"))
}

cfg := config.FromContextOrDefaults(ctx)
// Pipeline task having taskRef/taskSpec with APIVersion is classified as custom task
switch {
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
}
// PipelineTask must have a valid unique label and at least one of taskRef or taskSpec should be specified
errs = errs.Also(ValidatePipelineTasks(ctx, ps.Tasks, ps.Finally))
if len(ps.Resources) > 0 {
errs = errs.Also(apis.ErrDisallowedFields("resources"))
}
// Validate the pipeline task graph
errs = errs.Also(validateGraph(ps.Tasks))
// The parameter variables should be valid
Expand Down
41 changes: 41 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,47 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) {
Message: `missing field(s)`,
Paths: []string{"tasks[1].when[0]", "finally[0].when[0]"},
},
}, {
name: "uses resources",
ps: &PipelineSpec{
Resources: []PipelineDeclaredResource{{Name: "foo"}},
Tasks: []PipelineTask{{
Name: "valid-pipeline-task",
TaskRef: &TaskRef{Name: "foo-task"},
}},
},
expectedError: apis.FieldError{
Message: `must not set the field(s)`,
Paths: []string{"resources"},
},
}, {
name: "uses resources in tasks",
ps: &PipelineSpec{
Tasks: []PipelineTask{{
Name: "pipeline-task",
TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{Resources: &TaskResources{}, Steps: []Step{{Image: "my-image"}}}},
}},
},
expectedError: apis.FieldError{
Message: `must not set the field(s)`,
Paths: []string{"tasks[0].taskSpec.resources"},
},
}, {
name: "uses resources in finally",
ps: &PipelineSpec{
Tasks: []PipelineTask{{
Name: "valid-pipeline-task",
TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{Steps: []Step{{Image: "my-image"}}}},
}},
Finally: []PipelineTask{{
Name: "finally",
TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{Resources: &TaskResources{}, Steps: []Step{{Image: "my-image"}}}},
}},
},
expectedError: apis.FieldError{
Message: `must not set the field(s)`,
Paths: []string{"finally[0].taskSpec.resources"},
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,13 @@ type PipelineRunSpec struct {
PipelineRef *PipelineRef `json:"pipelineRef,omitempty"`
// +optional
PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"`
// Resources is a list of bindings specifying which actual instances of
// PipelineResources to use for the resources the Pipeline has declared
// it needs.
//
// Deprecated: Unused, preserved only for backwards compatibility
// +listType=atomic
Resources []PipelineResourceBinding `json:"resources,omitempty"`
// Params is a list of parameter names and values.
// +listType=atomic
Params Params `json:"params,omitempty"`
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError)
if ps.PodTemplate != nil {
errs = errs.Also(validatePodTemplateEnv(ctx, *ps.PodTemplate))
}
if ps.Resources != nil {
errs = errs.Also(apis.ErrDisallowedFields("resources"))
}

return errs
}
Expand Down
12 changes: 12 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,18 @@ func TestPipelineRun_Invalid(t *testing.T) {
Message: "invalid value: PipelineRun cannot be Pending after it is started",
Paths: []string{"spec.status"},
},
}, {
name: "uses resources",
pr: v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "pipelinerunname",
},
Spec: v1beta1.PipelineRunSpec{
PipelineRef: &v1beta1.PipelineRef{Name: "foo"},
Resources: []v1beta1.PipelineResourceBinding{{Name: "bar"}},
},
},
want: &apis.FieldError{Message: "must not set the field(s)", Paths: []string{"spec.resources"}},
}}

for _, tc := range tests {
Expand Down
Loading