Skip to content

Commit

Permalink
Revert removal of PipelineResources related fields
Browse files Browse the repository at this point in the history
This commit brings back PipelineResources related fields to the API structs,
marking them as deprecated and unused. The JSON tags are removed, so CRDs
cannot be defined with these fields. Keeping these fields in the Go structs
allows clients to maintain backwards compatibility with older servers, and
allows downstream projects such as chains to continue to work with older CRDs
that may define these fields.
  • Loading branch information
lbernick committed Mar 30, 2023
1 parent d3f8f3a commit 10709e8
Show file tree
Hide file tree
Showing 54 changed files with 5,726 additions and 100 deletions.
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

0 comments on commit 10709e8

Please sign in to comment.