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

Add a warning if git used as an output 🚨 #1310

Closed
wants to merge 1 commit into from
Closed
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
2 changes: 2 additions & 0 deletions docs/resources.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ the source code to be built by the pipeline. Adding the git resource as an input
to a Task will clone this repository and allow the Task to perform the required
actions on the contents of the repo.

_Git resources as outputs will soon not be supported, see https://github.com/tektoncd/pipeline/pull/1109._

To create a git resource using the `PipelineResource` CRD:

```yaml
Expand Down
4 changes: 4 additions & 0 deletions pkg/reconciler/taskrun/resources/output_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ func AddOutputResources(
return nil, xerrors.Errorf("failed to get output pipeline Resource for task %q resource %v", taskName, boundResource)
}

if resource.GetType() == v1alpha1.PipelineResourceTypeGit {
logger.Warn("The Task %s uses a git output: in the next release, support for git outputs will be removed (See #1109)")
}

var sourcePath string
if output.TargetPath == "" {
sourcePath = filepath.Join(outputDir, boundResource.Name)
Expand Down
54 changes: 54 additions & 0 deletions pkg/reconciler/taskrun/resources/output_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@ package resources
import (
"testing"

"go.uber.org/zap"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/logging"
"github.com/tektoncd/pipeline/test/names"
"go.uber.org/zap/zaptest/observer"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
fakek8s "k8s.io/client-go/kubernetes/fake"
Expand Down Expand Up @@ -777,6 +780,57 @@ func TestValidOutputResources(t *testing.T) {
}
}

// TestGitOutputWarns ensures we warn if git is used as an output.
// In #1109 we will remove support for the git resource as an output, until we add
// a different version via #1280 where a git output would create a new commit
func TestGitOutputWarns(t *testing.T) {
tr := &v1alpha1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "git-output-run",
},
Spec: v1alpha1.TaskRunSpec{
Outputs: v1alpha1.TaskRunOutputs{
Resources: []v1alpha1.TaskResourceBinding{{
Name: "some-repo",
ResourceRef: v1alpha1.PipelineResourceRef{
Name: "source-git",
},
}},
},
},
}
task := &v1alpha1.Task{
ObjectMeta: metav1.ObjectMeta{
Name: "task",
},
Spec: v1alpha1.TaskSpec{
Outputs: &v1alpha1.Outputs{
Resources: []v1alpha1.TaskResource{{
ResourceDeclaration: v1alpha1.ResourceDeclaration{
Name: "some-repo",
Type: "git",
}}},
},
},
}
r := map[string]v1alpha1.PipelineResourceInterface{
"some-repo": &v1alpha1.GitResource{
Name: "source-git",
},
}

outputResourceSetup(t)
o, ol := observer.New(zap.WarnLevel)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)

z := zap.New(o).Sugar()
_, err := AddOutputResources(fakek8s.NewSimpleClientset(), task.Name, &task.Spec, tr, r, z)
if err != nil {
t.Fatalf("Did not expect error but got %v", err)
}
if len(ol.All()) != 1 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be a different warning message, but I guess it's unlikely

t.Errorf("Expected to see a warning logged for a git output but saw none")
}
}

func TestValidOutputResourcesWithBucketStorage(t *testing.T) {
for _, c := range []struct {
name string
Expand Down