Skip to content

Commit

Permalink
Fix PipelineResource name handling.
Browse files Browse the repository at this point in the history
This commit changes the names used throughout PipelineResource handling.
PipelineResources have two names when used in a TaskRun:
- The name of the PipelineResource entity
- The name given to the resource inside the Task definition

Task code is currently using the external names everywhere, but should be using
the internal names.

Ref: #2694
  • Loading branch information
dlorenc committed May 27, 2020
1 parent 96c37b2 commit 23bbe1e
Show file tree
Hide file tree
Showing 29 changed files with 120 additions and 118 deletions.
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.13

require (
cloud.google.com/go v0.47.0 // indirect
cloud.google.com/go/storage v1.0.0
contrib.go.opencensus.io/exporter/stackdriver v0.12.8 // indirect
github.com/GoogleCloudPlatform/cloud-builders/gcs-fetcher v0.0.0-20191203181535-308b93ad1f39
github.com/cloudevents/sdk-go/v2 v2.0.0-RC3
Expand Down Expand Up @@ -38,6 +39,7 @@ require (
golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
golang.org/x/tools v0.0.0-20200214144324-88be01311a71 // indirect
gomodules.xyz/jsonpatch/v2 v2.1.0
google.golang.org/api v0.15.0
google.golang.org/appengine v1.6.5 // indirect
k8s.io/api v0.17.3
k8s.io/apiextensions-apiserver v0.17.3 // indirect
Expand Down
14 changes: 7 additions & 7 deletions pkg/apis/resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,20 @@ import (
// FromType returns an instance of the correct PipelineResource object type which can be
// used to add input and output containers as well as volumes to a TaskRun's pod in order to realize
// a PipelineResource in a pod.
func FromType(r *resourcev1alpha1.PipelineResource, images pipeline.Images) (pipelinev1beta1.PipelineResourceInterface, error) {
func FromType(name string, r *resourcev1alpha1.PipelineResource, images pipeline.Images) (pipelinev1beta1.PipelineResourceInterface, error) {
switch r.Spec.Type {
case resourcev1alpha1.PipelineResourceTypeGit:
return git.NewResource(images.GitImage, r)
return git.NewResource(name, images.GitImage, r)
case resourcev1alpha1.PipelineResourceTypeImage:
return image.NewResource(r)
return image.NewResource(name, r)
case resourcev1alpha1.PipelineResourceTypeCluster:
return cluster.NewResource(images.KubeconfigWriterImage, r)
return cluster.NewResource(name, images.KubeconfigWriterImage, r)
case resourcev1alpha1.PipelineResourceTypeStorage:
return storage.NewResource(images, r)
return storage.NewResource(name, images, r)
case resourcev1alpha1.PipelineResourceTypePullRequest:
return pullrequest.NewResource(images.PRImage, r)
return pullrequest.NewResource(name, images.PRImage, r)
case resourcev1alpha1.PipelineResourceTypeCloudEvent:
return cloudevent.NewResource(r)
return cloudevent.NewResource(name, r)
}
return nil, fmt.Errorf("%s is an invalid or unimplemented PipelineResource", r.Spec.Type)
}
4 changes: 2 additions & 2 deletions pkg/apis/resource/v1alpha1/cloudevent/cloud_event_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type Resource struct {
}

// NewResource creates a new CloudEvent resource to pass to a Task
func NewResource(r *resource.PipelineResource) (*Resource, error) {
func NewResource(name string, r *resource.PipelineResource) (*Resource, error) {
if r.Spec.Type != resource.PipelineResourceTypeCloudEvent {
return nil, fmt.Errorf("cloudevent.Resource: Cannot create a Cloud Event resource from a %s Pipeline Resource", r.Spec.Type)
}
Expand All @@ -55,7 +55,7 @@ func NewResource(r *resource.PipelineResource) (*Resource, error) {
return nil, fmt.Errorf("cloudevent.Resource: Need URI to be specified in order to create a CloudEvent resource %s", r.Name)
}
return &Resource{
Name: r.Name,
Name: name,
Type: r.Spec.Type,
TargetURI: targetURI,
}, nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestNewResource_Invalid(t *testing.T) {
}}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
_, err := cloudevent.NewResource(tc.pipelineResource)
_, err := cloudevent.NewResource("test-resource", tc.pipelineResource)
if err == nil {
t.Error("Expected error creating CloudEvent resource")
}
Expand All @@ -60,12 +60,12 @@ func TestNewResource_Valid(t *testing.T) {
tb.PipelineResourceSpecParam("TargetURI", "http://fake-sink"),
))
expectedResource := &cloudevent.Resource{
Name: "cloud-event-resource-uri",
Name: "test-resource",
TargetURI: "http://fake-sink",
Type: resourcev1alpha1.PipelineResourceTypeCloudEvent,
}

r, err := cloudevent.NewResource(pr)
r, err := cloudevent.NewResource("test-resource", pr)
if err != nil {
t.Fatalf("Unexpected error creating CloudEvent resource: %s", err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/resource/v1alpha1/cluster/cluster_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ type Resource struct {
}

// NewResource create a new k8s cluster resource to pass to a pipeline task
func NewResource(kubeconfigWriterImage string, r *resource.PipelineResource) (*Resource, error) {
func NewResource(name string, kubeconfigWriterImage string, r *resource.PipelineResource) (*Resource, error) {
if r.Spec.Type != resource.PipelineResourceTypeCluster {
return nil, fmt.Errorf("cluster.Resource: Cannot create a Cluster resource from a %s Pipeline Resource", r.Spec.Type)
}
clusterResource := Resource{
Type: r.Spec.Type,
KubeconfigWriterImage: kubeconfigWriterImage,
Name: r.Name,
Name: name,
}
for _, param := range r.Spec.Params {
switch {
Expand Down
26 changes: 13 additions & 13 deletions pkg/apis/resource/v1alpha1/cluster/cluster_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ func TestNewClusterResource(t *testing.T) {
want *cluster.Resource
}{{
desc: "basic cluster resource",
resource: tb.PipelineResource("test-cluster-resource", tb.PipelineResourceSpec(
resource: tb.PipelineResource("test-resource", tb.PipelineResourceSpec(
resourcev1alpha1.PipelineResourceTypeCluster,
tb.PipelineResourceSpecParam("url", "http://10.10.10.10"),
tb.PipelineResourceSpecParam("cadata", "bXktY2x1c3Rlci1jZXJ0Cg"),
tb.PipelineResourceSpecParam("token", "my-token"),
)),
want: &cluster.Resource{
Name: "test-cluster-resource",
Name: "test-resource",
Type: resourcev1alpha1.PipelineResourceTypeCluster,
URL: "http://10.10.10.10",
CAData: []byte("my-cluster-cert"),
Expand All @@ -52,15 +52,15 @@ func TestNewClusterResource(t *testing.T) {
},
}, {
desc: "resource with password instead of token",
resource: tb.PipelineResource("test-cluster-resource", tb.PipelineResourceSpec(
resource: tb.PipelineResource("test-resource", tb.PipelineResourceSpec(
resourcev1alpha1.PipelineResourceTypeCluster,
tb.PipelineResourceSpecParam("url", "http://10.10.10.10"),
tb.PipelineResourceSpecParam("cadata", "bXktY2x1c3Rlci1jZXJ0Cg"),
tb.PipelineResourceSpecParam("username", "user"),
tb.PipelineResourceSpecParam("password", "pass"),
)),
want: &cluster.Resource{
Name: "test-cluster-resource",
Name: "test-resource",
Type: resourcev1alpha1.PipelineResourceTypeCluster,
URL: "http://10.10.10.10",
CAData: []byte("my-cluster-cert"),
Expand All @@ -70,7 +70,7 @@ func TestNewClusterResource(t *testing.T) {
},
}, {
desc: "resource with clientKeyData and clientCertificateData instead of token or password",
resource: tb.PipelineResource("test-cluster-resource", tb.PipelineResourceSpec(
resource: tb.PipelineResource("test-resource", tb.PipelineResourceSpec(
resourcev1alpha1.PipelineResourceTypeCluster,
tb.PipelineResourceSpecParam("url", "http://10.10.10.10"),
tb.PipelineResourceSpecParam("username", "user"),
Expand All @@ -79,7 +79,7 @@ func TestNewClusterResource(t *testing.T) {
tb.PipelineResourceSpecParam("clientCertificateData", "Y2xpZW50LWNlcnRpZmljYXRlLWRhdGE="),
)),
want: &cluster.Resource{
Name: "test-cluster-resource",
Name: "test-resource",
Type: resourcev1alpha1.PipelineResourceTypeCluster,
URL: "http://10.10.10.10",
Username: "user",
Expand All @@ -90,13 +90,13 @@ func TestNewClusterResource(t *testing.T) {
},
}, {
desc: "set insecure flag to true when there is no cert",
resource: tb.PipelineResource("test-cluster-resource", tb.PipelineResourceSpec(
resource: tb.PipelineResource("test-resource", tb.PipelineResourceSpec(
resourcev1alpha1.PipelineResourceTypeCluster,
tb.PipelineResourceSpecParam("url", "http://10.10.10.10"),
tb.PipelineResourceSpecParam("token", "my-token"),
)),
want: &cluster.Resource{
Name: "test-cluster-resource",
Name: "test-resource",
Type: resourcev1alpha1.PipelineResourceTypeCluster,
URL: "http://10.10.10.10",
Token: "my-token",
Expand All @@ -105,15 +105,15 @@ func TestNewClusterResource(t *testing.T) {
},
}, {
desc: "basic cluster resource with namespace",
resource: tb.PipelineResource("test-cluster-resource", tb.PipelineResourceSpec(
resource: tb.PipelineResource("test-resource", tb.PipelineResourceSpec(
resourcev1alpha1.PipelineResourceTypeCluster,
tb.PipelineResourceSpecParam("url", "http://10.10.10.10"),
tb.PipelineResourceSpecParam("cadata", "bXktY2x1c3Rlci1jZXJ0Cg"),
tb.PipelineResourceSpecParam("token", "my-token"),
tb.PipelineResourceSpecParam("namespace", "my-namespace"),
)),
want: &cluster.Resource{
Name: "test-cluster-resource",
Name: "test-resource",
Type: resourcev1alpha1.PipelineResourceTypeCluster,
URL: "http://10.10.10.10",
CAData: []byte("my-cluster-cert"),
Expand All @@ -123,14 +123,14 @@ func TestNewClusterResource(t *testing.T) {
},
}, {
desc: "basic resource with secrets",
resource: tb.PipelineResource("test-cluster-resource", tb.PipelineResourceSpec(
resource: tb.PipelineResource("test-resource", tb.PipelineResourceSpec(
resourcev1alpha1.PipelineResourceTypeCluster,
tb.PipelineResourceSpecParam("url", "http://10.10.10.10"),
tb.PipelineResourceSpecSecretParam("cadata", "secret1", "cadatakey"),
tb.PipelineResourceSpecSecretParam("token", "secret1", "tokenkey"),
)),
want: &cluster.Resource{
Name: "test-cluster-resource",
Name: "test-resource",
Type: resourcev1alpha1.PipelineResourceTypeCluster,
URL: "http://10.10.10.10",
Secrets: []resourcev1alpha1.SecretParam{{
Expand All @@ -146,7 +146,7 @@ func TestNewClusterResource(t *testing.T) {
},
}} {
t.Run(c.desc, func(t *testing.T) {
got, err := cluster.NewResource("override-with-kubeconfig-writer:latest", c.resource)
got, err := cluster.NewResource("test-resource", "override-with-kubeconfig-writer:latest", c.resource)
if err != nil {
t.Errorf("Test: %q; TestNewClusterResource() error = %v", c.desc, err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/resource/v1alpha1/git/git_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ type Resource struct {
}

// NewResource creates a new git resource to pass to a Task
func NewResource(gitImage string, r *resource.PipelineResource) (*Resource, error) {
func NewResource(name, gitImage string, r *resource.PipelineResource) (*Resource, error) {
if r.Spec.Type != resource.PipelineResourceTypeGit {
return nil, fmt.Errorf("git.Resource: Cannot create a Git resource from a %s Pipeline Resource", r.Spec.Type)
}
gitResource := Resource{
Name: r.Name,
Name: name,
Type: r.Spec.Type,
GitImage: gitImage,
Submodules: true,
Expand Down
Loading

0 comments on commit 23bbe1e

Please sign in to comment.