[TEP074] Remove Git PipelineResources #6135

JeromeJu · 2023-02-08T23:27:22Z

This is targeted to be the last PR for the removal of PipelineResources along with the removal of generic pipelineresources functions.

Changes

This commit removes the Git pipelineResources, it removes github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/git and the corresponding test cases.

The coverage drops are expected as it is one of the last few PRs for removing the resources.

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

action required: please migrate off of `git` `pipelineresources` as it is removed, please refer to the doc at https://github.com/tektoncd/pipeline/blob/main/docs/pipelineresources.md#replacing-an-git-resource

tekton-robot · 2023-02-08T23:27:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign dibyom after the PR has been reviewed.
You can assign the PR to them by writing /assign @dibyom in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

JeromeJu · 2023-02-08T23:31:11Z

/kind misc

tekton-robot · 2023-02-08T23:33:07Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	87.2%	86.5%	-0.8
pkg/reconciler/pipelinerun/resources/input_output_steps.go	96.9%	84.4%	-12.5
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	94.9%	94.7%	-0.3
pkg/reconciler/taskrun/resources/taskresourceresolution.go	100.0%	89.5%	-10.5
pkg/reconciler/taskrun/taskrun.go	84.5%	83.8%	-0.7
pkg/reconciler/taskrun/validate_resources.go	98.5%	97.8%	-0.7

tekton-robot · 2023-02-08T23:36:08Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	87.2%	86.5%	-0.8
pkg/reconciler/pipelinerun/resources/input_output_steps.go	96.9%	84.4%	-12.5
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	94.9%	94.7%	-0.3
pkg/reconciler/taskrun/resources/taskresourceresolution.go	100.0%	89.5%	-10.5
pkg/reconciler/taskrun/taskrun.go	84.5%	83.8%	-0.7
pkg/reconciler/taskrun/validate_resources.go	98.5%	97.8%	-0.7

tekton-robot · 2023-02-08T23:37:55Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	87.2%	86.5%	-0.8
pkg/reconciler/pipelinerun/resources/input_output_steps.go	96.9%	84.4%	-12.5
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	94.9%	94.7%	-0.3
pkg/reconciler/taskrun/resources/taskresourceresolution.go	100.0%	89.5%	-10.5
pkg/reconciler/taskrun/taskrun.go	84.5%	83.8%	-0.7
pkg/reconciler/taskrun/validate_resources.go	98.5%	97.8%	-0.7

tekton-robot · 2023-02-09T01:51:13Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	87.2%	86.5%	-0.8
pkg/reconciler/pipelinerun/resources/input_output_steps.go	96.9%	84.4%	-12.5
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	94.9%	94.7%	-0.3
pkg/reconciler/taskrun/resources/taskresourceresolution.go	100.0%	89.5%	-10.5
pkg/reconciler/taskrun/taskrun.go	84.5%	83.8%	-0.7
pkg/reconciler/taskrun/validate_resources.go	98.5%	97.8%	-0.7

tekton-robot · 2023-02-09T01:53:39Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	87.2%	86.5%	-0.8
pkg/reconciler/pipelinerun/resources/input_output_steps.go	96.9%	84.4%	-12.5
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	94.9%	94.7%	-0.3
pkg/reconciler/taskrun/resources/taskresourceresolution.go	100.0%	89.5%	-10.5
pkg/reconciler/taskrun/taskrun.go	84.5%	83.8%	-0.7
pkg/reconciler/taskrun/validate_resources.go	98.5%	97.8%	-0.7

tekton-robot · 2023-02-09T02:56:04Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	87.2%	86.5%	-0.8
pkg/reconciler/pipelinerun/resources/input_output_steps.go	96.9%	84.4%	-12.5
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	94.9%	94.7%	-0.3
pkg/reconciler/taskrun/resources/taskresourceresolution.go	100.0%	89.5%	-10.5
pkg/reconciler/taskrun/taskrun.go	84.5%	83.8%	-0.7
pkg/reconciler/taskrun/validate_resources.go	98.5%	97.8%	-0.7

tekton-robot · 2023-02-09T02:58:25Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	87.2%	86.5%	-0.8
pkg/reconciler/pipelinerun/resources/input_output_steps.go	96.9%	84.4%	-12.5
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	94.9%	94.7%	-0.3
pkg/reconciler/taskrun/resources/taskresourceresolution.go	100.0%	89.5%	-10.5
pkg/reconciler/taskrun/taskrun.go	84.5%	83.8%	-0.7
pkg/reconciler/taskrun/validate_resources.go	98.5%	97.8%	-0.7

pkg/reconciler/pipelinerun/pipelinerun_test.go

This commit removes the `Git` pipelineResources, it removes `github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/git` and the corresponding test cases.

tekton-robot · 2023-02-09T21:12:22Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	87.2%	86.5%	-0.8
pkg/reconciler/pipelinerun/resources/input_output_steps.go	96.9%	84.4%	-12.5
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	94.9%	94.7%	-0.3
pkg/reconciler/taskrun/resources/taskresourceresolution.go	100.0%	89.5%	-10.5
pkg/reconciler/taskrun/taskrun.go	84.5%	83.8%	-0.7
pkg/reconciler/taskrun/validate_resources.go	98.5%	97.8%	-0.7

tekton-robot · 2023-02-09T21:14:57Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	87.2%	86.5%	-0.8
pkg/reconciler/pipelinerun/resources/input_output_steps.go	96.9%	84.4%	-12.5
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	94.9%	94.7%	-0.3
pkg/reconciler/taskrun/resources/taskresourceresolution.go	100.0%	89.5%	-10.5
pkg/reconciler/taskrun/taskrun.go	84.5%	83.8%	-0.7
pkg/reconciler/taskrun/validate_resources.go	98.5%	97.8%	-0.7

lbernick · 2023-02-10T13:08:54Z

pkg/apis/pipeline/v1beta1/task_validation_test.go

-var validResource = v1beta1.TaskResource{
-	ResourceDeclaration: v1beta1.ResourceDeclaration{
-		Name: "validsource",
-		Type: "git",


technically it would be better to replace this with the image resource as you've done elsewhere (aligns better with our code standards on always being in a releasable state)

Thanks for the suggestion, I was also debating this to see what would be a best solution since we are going to be in a stage where all the resourceTypes are removed before we start to remove the generic resourceTypes functions. Does that mean the removal of the last resourceTypes should also include the removal of the generic functions? But until then we need to change them all to the very last resource type?

Thanks for the answer offline with Lee.

I am hoping to put this PR as the last removal along with the generic functions of PipelineResources eg. inputresources etc.

It might make sense to put a hold on this pr or mark it as a draft in that case

lbernick · 2023-02-10T13:11:29Z

pkg/artifacts/artifact_storage_test.go

@@ -469,31 +173,6 @@ func TestCleanupArtifactStorage(t *testing.T) {
 	}
 }

-func TestInitializeArtifactStorageWithoutConfigMap(t *testing.T) {


is the function InitializeArtifactStorage deleted somewhere?

lbernick · 2023-02-10T13:12:54Z

pkg/reconciler/taskrun/resources/apply_test.go

@@ -1103,19 +1086,6 @@ func TestApplyResources(t *testing.T) {
 			spec.Steps[8].Image = "/foo/builtImage"
 			spec.Steps[9].Image = "/workspace/foo/builtImage"
 		}),
-	}, {
-		name: "input resource specified",


I think this is OK to delete fully because despite the test name it's really a test for the git pipelineresource mutations it looks like

lbernick · 2023-02-10T13:13:28Z

pkg/reconciler/taskrun/resources/input_resource_test.go

 	gcsInputs = []v1beta1.TaskResource{{
 		ResourceDeclaration: v1beta1.ResourceDeclaration{
 			Name:       "workspace",
 			Type:       "gcs",
 			TargetPath: "gcs-dir",
 		}}}
-	multipleGcsInputs = []v1beta1.TaskResource{


should this be restored?

lbernick · 2023-02-10T13:14:02Z

pkg/reconciler/taskrun/resources/input_resource_test.go

-	}, {
-		desc: "invalid gcs resource type name",
-		task: task,
-		taskRun: &v1beta1.TaskRun{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "get-from-invalid-gcs",
-				Namespace: "marshmallow",
-			},
-			Spec: v1beta1.TaskRunSpec{
-				Resources: &v1beta1.TaskRunResources{
-					Inputs: []v1beta1.TaskResourceBinding{{
-						PipelineResourceBinding: v1beta1.PipelineResourceBinding{
-							ResourceRef: &v1beta1.PipelineResourceRef{
-								Name: "storage-gcs-invalid",
-							},
-							Name: "workspace",
-						},
-					}},
-				},
-			},
-		},
-		wantErr: true,
-	}, {
-		desc: "invalid gcs resource type name",
-		task: task,
-		taskRun: &v1beta1.TaskRun{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "get-from-invalid-gcs",
-				Namespace: "marshmallow",
-			},
-			Spec: v1beta1.TaskRunSpec{
-				Resources: &v1beta1.TaskRunResources{
-					Inputs: []v1beta1.TaskResourceBinding{{
-						PipelineResourceBinding: v1beta1.PipelineResourceBinding{
-							ResourceRef: &v1beta1.PipelineResourceRef{
-								Name: "storage-gcs-invalid",
-							},
-							Name: "workspace",
-						},
-					}},
-				},
-			},
-		},
-		wantErr: true,


should this be restored?

lbernick · 2023-02-10T13:14:39Z

pkg/reconciler/taskrun/resources/input_resource_test.go

-			},
-		},
-	}
-	taskWithMultipleGcsInputs := &v1beta1.Task{


curious why this PR deletes a lot of GCS tests?

lbernick · 2023-02-10T13:15:43Z

pkg/reconciler/taskrun/resources/taskresourceresolution_test.go

-				Name: "git-repo",
-			},
-			ResourceSpec: &resourcev1alpha1.PipelineResourceSpec{
-				Type: resourcev1alpha1.PipelineResourceTypeGit,


I'd swap to a different resource type here

JeromeJu · 2023-02-10T18:07:14Z

/hold
This will be the last removal of pipelineResources along with the generic functions.

JeromeJu · 2023-02-13T16:15:29Z

Closing as #6150 supersedes this PR

tekton-robot added the release-note-none Denotes a PR that doesnt merit a release note. label Feb 8, 2023

tekton-robot requested review from lbernick and wlynch February 8, 2023 23:27

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 8, 2023

JeromeJu force-pushed the rm-git-resources branch from e5d65a1 to eefc5a2 Compare February 8, 2023 23:28

tekton-robot added the kind/misc Categorizes issue or PR as a miscellaneuous one. label Feb 8, 2023

JeromeJu force-pushed the rm-git-resources branch from eefc5a2 to 7f39c57 Compare February 9, 2023 01:45

tekton-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. and removed release-note-none Denotes a PR that doesnt merit a release note. labels Feb 9, 2023

JeromeJu commented Feb 9, 2023

View reviewed changes

pkg/reconciler/pipelinerun/pipelinerun_test.go Outdated Show resolved Hide resolved

JeromeJu mentioned this pull request Feb 9, 2023

Refactor getTaskRunCreations to be explicit on getting TaskRuns #6143

Closed

JeromeJu changed the title ~~Remove Git PipelineResources~~ [TEP074] Remove Git PipelineResources Feb 9, 2023

Remove Git PipelineResources

3999819

This commit removes the `Git` pipelineResources, it removes `github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/git` and the corresponding test cases.

JeromeJu force-pushed the rm-git-resources branch from 6de3871 to 3999819 Compare February 9, 2023 21:07

lbernick reviewed Feb 10, 2023

View reviewed changes

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 10, 2023

JeromeJu closed this Feb 13, 2023

JeromeJu mentioned this pull request Feb 13, 2023

Handle PipelineResources when swapping v1 to the storage version #5967

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TEP074] Remove Git PipelineResources #6135

[TEP074] Remove Git PipelineResources #6135

JeromeJu commented Feb 8, 2023 •

edited

Loading

tekton-robot commented Feb 8, 2023

JeromeJu commented Feb 8, 2023

tekton-robot commented Feb 8, 2023

tekton-robot commented Feb 8, 2023

tekton-robot commented Feb 8, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

lbernick Feb 10, 2023

JeromeJu Feb 10, 2023

JeromeJu Feb 10, 2023

lbernick Feb 10, 2023

lbernick Feb 10, 2023

lbernick Feb 10, 2023

lbernick Feb 10, 2023

lbernick Feb 10, 2023

lbernick Feb 10, 2023

lbernick Feb 10, 2023

JeromeJu commented Feb 10, 2023

JeromeJu commented Feb 13, 2023

[TEP074] Remove Git PipelineResources #6135

[TEP074] Remove Git PipelineResources #6135

Conversation

JeromeJu commented Feb 8, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Feb 8, 2023

JeromeJu commented Feb 8, 2023

tekton-robot commented Feb 8, 2023

tekton-robot commented Feb 8, 2023

tekton-robot commented Feb 8, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

tekton-robot commented Feb 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeromeJu commented Feb 10, 2023

JeromeJu commented Feb 13, 2023

JeromeJu commented Feb 8, 2023 •

edited

Loading