Skip to content

Commit

Permalink
Refactor functions to not be members of Sink 🚰
Browse files Browse the repository at this point in the history
Just like with reconcilers / controllers, adding functions of as members
of the Sink (or controller) object itself has some downsides: it's hard
to tell what the responsibilities of the functions are and how they are
coupled to the Sink, and it's harder to write more focused unit tests.

I am working on tektoncd#258 in another branch and in that branch I wanted to
make some updates to the unit tests for this logic; it's easier to do if
it's factored out a bit.

There is more factoring we can do here: now that the functions are moved
out of Sink we can make decisions around their interfaces and maybe make
them even more focused and unit testable.

(Though it was a happy surprise to see they were unit tested even though
they were members of Sink!)

Also change usage of xerrors to fmt.Errorf since we're using >= go 1.13
now and we get the error wrapping for free :D
  • Loading branch information
bobcatfish committed Dec 12, 2019
1 parent e6881e2 commit daa92c7
Show file tree
Hide file tree
Showing 5 changed files with 476 additions and 383 deletions.
118 changes: 118 additions & 0 deletions pkg/resources/create.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
Copyright 2019 The Tekton Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package resources

import (
"encoding/json"
"fmt"
"strings"

"k8s.io/client-go/dynamic"

"go.uber.org/zap"

triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
"github.com/tidwall/sjson"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
discoveryclient "k8s.io/client-go/discovery"
)

// FindAPIResource returns the APIResource definition using the discovery client c.
func FindAPIResource(apiVersion, kind string, c discoveryclient.ServerResourcesInterface) (*metav1.APIResource, error) {
resourceList, err := c.ServerResourcesForGroupVersion(apiVersion)
if err != nil {
return nil, fmt.Errorf("Error getting kubernetes server resources for apiVersion %s: %s", apiVersion, err)
}
for _, apiResource := range resourceList.APIResources {
if apiResource.Kind != kind {
continue
}
r := &apiResource
// Resolve GroupVersion from parent list to have consistent resource identifiers.
if r.Version == "" || r.Group == "" {
gv, err := schema.ParseGroupVersion(resourceList.GroupVersion)
if err != nil {
return nil, fmt.Errorf("error parsing parsing GroupVersion: %v", err)
}
r.Group = gv.Group
r.Version = gv.Version
}
return r, nil
}
return nil, fmt.Errorf("Error could not find resource with apiVersion %s and kind %s", apiVersion, kind)
}

// Create uses the kubeClient to create the resource defined in the
// TriggerResourceTemplate and returns any errors with this process
func Create(logger *zap.SugaredLogger, rt json.RawMessage, triggerName, eventID, elName, elNamespace string, c discoveryclient.ServerResourcesInterface, dc dynamic.Interface) error {
rt, err := AddLabels(rt, map[string]string{
triggersv1.EventListenerLabelKey: elName,
triggersv1.EventIDLabelKey: eventID,
triggersv1.TriggerLabelKey: triggerName,
})
if err != nil {
return err
}

// Assume the TriggerResourceTemplate is valid (it has an apiVersion and Kind)
data := new(unstructured.Unstructured)
if err := data.UnmarshalJSON(rt); err != nil {
return err
}

namespace := data.GetNamespace()
// Default the resource creation to the EventListenerNamespace if not found in the resource template
if namespace == "" {
namespace = elNamespace
}

// Resolve resource kind to the underlying API Resource type.
apiResource, err := FindAPIResource(data.GetAPIVersion(), data.GetKind(), c)
if err != nil {
return err
}

name := data.GetName()
if name == "" {
name = data.GetGenerateName()
}
logger.Infof("Generating resource: kind: %+v, name: %s", apiResource, name)

gvr := schema.GroupVersionResource{
Group: apiResource.Group,
Version: apiResource.Version,
Resource: apiResource.Name,
}

_, err = dc.Resource(gvr).Namespace(namespace).Create(data, metav1.CreateOptions{})
return err
}

// AddLabels adds autogenerated Tekton labels to created resources.
func AddLabels(rt json.RawMessage, labels map[string]string) (json.RawMessage, error) {
var err error
for k, v := range labels {
l := fmt.Sprintf("metadata.labels.%s/%s", triggersv1.LabelEscape, strings.TrimLeft(k, "/"))
rt, err = sjson.SetBytes(rt, l, v)
if err != nil {
return rt, err
}
}
return rt, err
}
280 changes: 280 additions & 0 deletions pkg/resources/create_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,280 @@
/*
Copyright 2019 The Tekton Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package resources

import (
"encoding/json"
"fmt"
"testing"

"github.com/google/go-cmp/cmp"
pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
dynamicclientset "github.com/tektoncd/triggers/pkg/client/dynamic/clientset"
"github.com/tektoncd/triggers/pkg/client/dynamic/clientset/tekton"
"github.com/tektoncd/triggers/test"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
fakedynamic "k8s.io/client-go/dynamic/fake"
fakekubeclientset "k8s.io/client-go/kubernetes/fake"
ktesting "k8s.io/client-go/testing"
"knative.dev/pkg/logging"
)

const (
resourceLabel = triggersv1.GroupName + triggersv1.EventListenerLabelKey
triggerLabel = triggersv1.GroupName + triggersv1.TriggerLabelKey
eventIDLabel = triggersv1.GroupName + triggersv1.EventIDLabelKey

triggerName = "trigger"
eventID = "12345"
)

func Test_FindAPIResource_error(t *testing.T) {
dc := fakekubeclientset.NewSimpleClientset().Discovery()
if _, err := FindAPIResource("v1", "Pod", dc); err == nil {
t.Error("findAPIResource() did not return error when expected")
}
}

func TestFindAPIResource(t *testing.T) {
// Create fake kubeclient with list of resources
kubeClient := fakekubeclientset.NewSimpleClientset()
kubeClient.Resources = []*metav1.APIResourceList{{
GroupVersion: "v1",
APIResources: []metav1.APIResource{{
Name: "pods",
Namespaced: true,
Kind: "Pod",
}, {
Name: "namespaces",
Namespaced: false,
Kind: "Namespace",
}},
}}
test.AddTektonResources(kubeClient)
dc := kubeClient.Discovery()

tests := []struct {
apiVersion string
kind string
want *metav1.APIResource
}{{
apiVersion: "v1",
kind: "Pod",
want: &metav1.APIResource{
Name: "pods",
Namespaced: true,
Version: "v1",
Kind: "Pod",
},
}, {
apiVersion: "v1",
kind: "Namespace",
want: &metav1.APIResource{
Name: "namespaces",
Namespaced: false,
Version: "v1",
Kind: "Namespace",
},
}, {
apiVersion: "tekton.dev/v1alpha1",
kind: "TriggerTemplate",
want: &metav1.APIResource{
Group: "tekton.dev",
Version: "v1alpha1",
Name: "triggertemplates",
Namespaced: true,
Kind: "TriggerTemplate",
},
}, {
apiVersion: "tekton.dev/v1alpha1",
kind: "PipelineRun",
want: &metav1.APIResource{
Group: "tekton.dev",
Version: "v1alpha1",
Name: "pipelineruns",
Namespaced: true,
Kind: "PipelineRun",
},
},
}
for _, tt := range tests {
t.Run(fmt.Sprintf("%s_%s", tt.apiVersion, tt.kind), func(t *testing.T) {
got, err := FindAPIResource(tt.apiVersion, tt.kind, dc)
if err != nil {
t.Errorf("findAPIResource() returned error: %s", err)
} else if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("findAPIResource() Diff: -want +got: %s", diff)
}
})
}
}

func TestCreateResource(t *testing.T) {
elName := "foo-el"
elNamespace := "bar"

kubeClient := fakekubeclientset.NewSimpleClientset()
test.AddTektonResources(kubeClient)

dynamicClient := fakedynamic.NewSimpleDynamicClient(runtime.NewScheme())
dynamicSet := dynamicclientset.New(tekton.WithClient(dynamicClient))

logger, _ := logging.NewLogger("", "")

tests := []struct {
name string
resource pipelinev1.PipelineResource
want pipelinev1.PipelineResource
}{{
name: "PipelineResource without namespace",
resource: pipelinev1.PipelineResource{
TypeMeta: metav1.TypeMeta{
APIVersion: "tekton.dev/v1alpha1",
Kind: "PipelineResource",
},
ObjectMeta: metav1.ObjectMeta{
Name: "my-pipelineresource",
Labels: map[string]string{"woriginal-label-1": "label-1"},
},
Spec: pipelinev1.PipelineResourceSpec{},
},
want: pipelinev1.PipelineResource{
TypeMeta: metav1.TypeMeta{
APIVersion: "tekton.dev/v1alpha1",
Kind: "PipelineResource",
},
ObjectMeta: metav1.ObjectMeta{
Name: "my-pipelineresource",
Labels: map[string]string{
"woriginal-label-1": "label-1",
resourceLabel: elName,
triggerLabel: triggerName,
eventIDLabel: eventID,
},
},
Spec: pipelinev1.PipelineResourceSpec{},
},
}, {
name: "PipelineResource with namespace",
resource: pipelinev1.PipelineResource{
TypeMeta: metav1.TypeMeta{
APIVersion: "tekton.dev/v1alpha1",
Kind: "PipelineResource",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "my-pipelineresource",
Labels: map[string]string{"woriginal-label-1": "label-1"},
},
Spec: pipelinev1.PipelineResourceSpec{},
},
want: pipelinev1.PipelineResource{
TypeMeta: metav1.TypeMeta{
APIVersion: "tekton.dev/v1alpha1",
Kind: "PipelineResource",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "my-pipelineresource",
Labels: map[string]string{
"woriginal-label-1": "label-1",
resourceLabel: elName,
triggerLabel: triggerName,
eventIDLabel: eventID,
},
},
Spec: pipelinev1.PipelineResourceSpec{},
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
dynamicClient.ClearActions()

b, err := json.Marshal(tt.resource)
if err != nil {
t.Fatalf("error marshalling resource: %v", tt.resource)
}
if err := Create(logger, b, triggerName, eventID, elName, elNamespace, kubeClient.Discovery(), dynamicSet); err != nil {
t.Errorf("createResource() returned error: %s", err)
}

gvr := schema.GroupVersionResource{
Group: "tekton.dev",
Version: "v1alpha1",
Resource: "pipelineresources",
}
namespace := tt.want.Namespace
if namespace == "" {
namespace = elNamespace
}
want := []ktesting.Action{ktesting.NewCreateAction(gvr, namespace, test.ToUnstructured(t, tt.want))}
if diff := cmp.Diff(want, dynamicClient.Actions()); diff != "" {
t.Error(diff)
}
})
}
}

func Test_AddLabels(t *testing.T) {
b, err := json.Marshal(map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
// should be overwritten
"tekton.dev/a": "0",
// should be preserved.
"tekton.dev/z": "0",
"best-palindrome": "tacocat",
},
},
})
if err != nil {
t.Fatalf("json.Marshal: %v", err)
}

raw, err := AddLabels(json.RawMessage(b), map[string]string{
"a": "1",
"/b": "2",
"//c": "3",
})
if err != nil {
t.Fatalf("addLabels: %v", err)
}

got := make(map[string]interface{})
if err := json.Unmarshal(raw, &got); err != nil {
t.Fatalf("json.Unmarshal: %v", err)
}

want := map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"tekton.dev/a": "1",
"tekton.dev/b": "2",
"tekton.dev/c": "3",
"tekton.dev/z": "0",
"best-palindrome": "tacocat",
},
},
}

if diff := cmp.Diff(want, got); diff != "" {
t.Error(diff)
}
}
Loading

0 comments on commit daa92c7

Please sign in to comment.