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

Make ResourceRef in PipelineResourceBinding consistent with TaskRun #1497

Merged
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
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1alpha1/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ type PipelineResource struct {
metav1.ObjectMeta `json:"metadata,omitempty"`

// Spec holds the desired state of the PipelineResource from the client
// +optional
Spec PipelineResourceSpec `json:"spec,omitempty"`
// Status communicates the observed state of the PipelineResource from the controller
// +optional
Expand All @@ -163,10 +162,11 @@ type PipelineResourceBinding struct {
Name string `json:"name,omitempty"`
// ResourceRef is a reference to the instance of the actual PipelineResource
// that should be used
ResourceRef PipelineResourceRef `json:"resourceRef,omitempty"`
// +optional
ResourceRef *PipelineResourceRef `json:"resourceRef,omitempty"`
// ResourceSpec is specification of a resource that should be created and
// consumed by the task
// +optional
ResourceSpec *PipelineResourceSpec `json:"resourceSpec,omitempty"`
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1alpha1/taskrun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ func validatePipelineResources(ctx context.Context, resources []TaskResourceBind
}
encountered[name] = struct{}{}
// Check that both resource ref and resource Spec are not present
if r.ResourceRef.Name != "" && r.ResourceSpec != nil {
if r.ResourceRef != nil && r.ResourceSpec != nil {
return apis.ErrDisallowedFields(fmt.Sprintf("%s.ResourceRef", path), fmt.Sprintf("%s.ResourceSpec", path))
}
// Check that one of resource ref and resource Spec is present
if r.ResourceRef.Name == "" && r.ResourceSpec == nil {
if (r.ResourceRef == nil || r.ResourceRef.Name == "") && r.ResourceSpec == nil {
return apis.ErrMissingField(fmt.Sprintf("%s.ResourceRef", path), fmt.Sprintf("%s.ResourceSpec", path))
}
if r.ResourceSpec != nil && r.ResourceSpec.Validate(ctx) != nil {
Expand Down
16 changes: 8 additions & 8 deletions pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func TestInput_Validate(t *testing.T) {
}},
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "testresource",
},
Name: "workspace",
Expand All @@ -195,14 +195,14 @@ func TestInput_Invalidate(t *testing.T) {
inputs: v1alpha1.TaskRunInputs{
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "testresource1",
},
Name: "workspace",
},
}, {
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "testresource2",
},
Name: "workspace",
Expand All @@ -215,7 +215,7 @@ func TestInput_Invalidate(t *testing.T) {
inputs: v1alpha1.TaskRunInputs{
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "testresource",
},
Name: "resource",
Expand All @@ -235,7 +235,7 @@ func TestInput_Invalidate(t *testing.T) {
inputs: v1alpha1.TaskRunInputs{
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "testresource",
},
ResourceSpec: &v1alpha1.PipelineResourceSpec{
Expand Down Expand Up @@ -284,7 +284,7 @@ func TestOutput_Validate(t *testing.T) {
i := v1alpha1.TaskRunOutputs{
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "testresource",
},
Name: "someimage",
Expand All @@ -305,14 +305,14 @@ func TestOutput_Invalidate(t *testing.T) {
outputs: v1alpha1.TaskRunOutputs{
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "testresource1",
},
Name: "workspace",
},
}, {
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "testresource2",
},
Name: "workspace",
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (rcc *ResolvedConditionCheck) ToTaskResourceBindings() []v1alpha1.TaskResou
},
}
if r.SelfLink != "" {
tr.ResourceRef = v1alpha1.PipelineResourceRef{
tr.ResourceRef = &v1alpha1.PipelineResourceRef{
Name: r.Name,
APIVersion: r.APIVersion,
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/pipelinerun/resources/input_output_steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func GetOutputSteps(outputs map[string]*v1alpha1.PipelineResource, taskName, sto
// SelfLink is being checked there to determine if this PipelineResource is an instance that
// exists in the cluster (in which case Kubernetes will populate this field) or is specified by Spec
if outputResource.SelfLink != "" {
taskOutputResource.ResourceRef = v1alpha1.PipelineResourceRef{
taskOutputResource.ResourceRef = &v1alpha1.PipelineResourceRef{
Name: outputResource.Name,
APIVersion: outputResource.APIVersion,
}
Expand Down Expand Up @@ -66,7 +66,7 @@ func GetInputSteps(inputs map[string]*v1alpha1.PipelineResource, pt *v1alpha1.Pi
// SelfLink is being checked there to determine if this PipelineResource is an instance that
// exists in the cluster (in which case Kubernetes will populate this field) or is specified by Spec
if inputResource.SelfLink != "" {
taskInputResource.ResourceRef = v1alpha1.PipelineResourceRef{
taskInputResource.ResourceRef = &v1alpha1.PipelineResourceRef{
Name: inputResource.Name,
APIVersion: inputResource.APIVersion,
}
Expand Down
18 changes: 9 additions & 9 deletions pkg/reconciler/pipelinerun/resources/input_output_steps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestGetOutputSteps(t *testing.T) {
expectedtaskOuputResources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
Name: "test-output",
ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
},
Paths: []string{"/pvc/test-taskname/test-output"},
}},
Expand All @@ -76,13 +76,13 @@ func TestGetOutputSteps(t *testing.T) {
expectedtaskOuputResources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
Name: "test-output",
ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
},
Paths: []string{"/pvc/test-multiple-outputs/test-output"},
}, {
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
Name: "test-output-2",
ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource2"},
ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource2"},
},
Paths: []string{"/pvc/test-multiple-outputs/test-output-2"},
}},
Expand Down Expand Up @@ -166,7 +166,7 @@ func TestGetInputSteps(t *testing.T) {
},
expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
Name: "test-input",
},
Paths: []string{"/pvc/prev-task-1/test-input"},
Expand All @@ -176,7 +176,7 @@ func TestGetInputSteps(t *testing.T) {
inputs: map[string]*v1alpha1.PipelineResource{"test-input": r1},
expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
Name: "test-input",
},
}},
Expand All @@ -196,7 +196,7 @@ func TestGetInputSteps(t *testing.T) {
},
expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
Name: "test-input",
},
Paths: []string{"/pvc/prev-task-1/test-input", "/pvc/prev-task-2/test-input"},
Expand Down Expand Up @@ -305,13 +305,13 @@ func TestWrapSteps(t *testing.T) {

expectedtaskInputResources := []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
Name: "test-input",
},
Paths: []string{"/pvc/prev-task/test-input"},
}, {
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
Name: "test-input-2",
},
}, {
Expand All @@ -322,7 +322,7 @@ func TestWrapSteps(t *testing.T) {
}}
expectedtaskOuputResources := []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
Name: "test-output",
},
Paths: []string{"/pvc/test-task/test-output"},
Expand Down
8 changes: 4 additions & 4 deletions pkg/reconciler/taskrun/resources/image_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestAddOutputImageDigestExporter(t *testing.T) {
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
Name: "source-image",
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "source-image-1",
},
},
Expand All @@ -80,7 +80,7 @@ func TestAddOutputImageDigestExporter(t *testing.T) {
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
Name: "source-image",
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "source-image-1",
},
},
Expand Down Expand Up @@ -137,7 +137,7 @@ func TestAddOutputImageDigestExporter(t *testing.T) {
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
Name: "source-image",
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "source-image-1",
},
},
Expand All @@ -147,7 +147,7 @@ func TestAddOutputImageDigestExporter(t *testing.T) {
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
Name: "source-image",
ResourceRef: v1alpha1.PipelineResourceRef{
ResourceRef: &v1alpha1.PipelineResourceRef{
Name: "source-image-1",
},
},
Expand Down
Loading