Skip to content

Commit

Permalink
Remove the custom "name" parameter from cluster resources.
Browse files Browse the repository at this point in the history
This is redundant with the Name that Tasks already declare on Resources.
The name is used both to name the cluster in the kubeconfig and to set the
path for the kubeconfig. Using a name on the resource itself means Tasks can't
know where to expect this file.
  • Loading branch information
dlorenc committed Oct 25, 2019
1 parent 6072ed9 commit d52ebb8
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 17 deletions.
4 changes: 0 additions & 4 deletions docs/resources.md
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,6 @@ cluster. The kubeconfig will be placed in

The Cluster resource has the following parameters:

- `name` (required): The name to be given to the target cluster, will be used
in the kubeconfig and also as part of the path to the kubeconfig file
- `url` (required): Host url of the master node
- `username` (required): the user with access to the cluster
- `password`: to be used for clusters with basic auth
Expand Down Expand Up @@ -671,8 +669,6 @@ spec:
params:
- name: url
value: https://<ip address determined above>
- name: name
value: mycluster
secrets:
- fieldName: cadata
secretName: cluster-ca-data
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/pipeline/v1alpha1/cluster_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ func NewClusterResource(kubeconfigWriterImage string, r *PipelineResource) (*Clu
clusterResource := ClusterResource{
Type: r.Spec.Type,
KubeconfigWriterImage: kubeconfigWriterImage,
Name: r.Name,
}
for _, param := range r.Spec.Params {
switch {
case strings.EqualFold(param.Name, "Name"):
clusterResource.Name = param.Value
clusterResource.Name = param.Name
case strings.EqualFold(param.Name, "URL"):
clusterResource.URL = param.Value
case strings.EqualFold(param.Name, "Revision"):
Expand Down
6 changes: 4 additions & 2 deletions pkg/apis/pipeline/v1alpha1/pipelineresource_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"k8s.io/apimachinery/pkg/api/equality"
"knative.dev/pkg/apis"
logging "knative.dev/pkg/logging"
)

func (r *PipelineResource) Validate(ctx context.Context) *apis.FieldError {
Expand Down Expand Up @@ -71,8 +72,9 @@ func (rs *PipelineResourceSpec) Validate(ctx context.Context) *apis.FieldError {
}
}

if !nameFound {
return apis.ErrMissingField("name param")
if nameFound {
logging.FromContext(ctx).Warn(
"The name parameter on the cluster resource is deprecated. Support will be removed in a future release")
}
// One auth method must be supplied
if !(authFound) {
Expand Down
10 changes: 0 additions & 10 deletions pkg/apis/pipeline/v1alpha1/pipelineresource_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,6 @@ func TestResourceValidation_Invalid(t *testing.T) {
)),
want: apis.ErrMissingField("username or CAData or token param"),
},
{
name: "cluster with missing name",
res: tb.PipelineResource("test-cluster-resource", "foo", tb.PipelineResourceSpec(
v1alpha1.PipelineResourceTypeCluster,
tb.PipelineResourceSpecParam("url", "http://10.10.10.10"),
tb.PipelineResourceSpecParam("cadata", "bXktY2x1c3Rlci1jZXJ0Cg"),
tb.PipelineResourceSpecParam("token", "my-token"),
)),
want: apis.ErrMissingField("name param"),
},
{
name: "cluster with missing cadata",
res: tb.PipelineResource("test-cluster-resource", "foo", tb.PipelineResourceSpec(
Expand Down

0 comments on commit d52ebb8

Please sign in to comment.