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.

Sign up for GitHub

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

Switch to UUID for event IDs #926

Merged
merged 1 commit into from
Jan 26, 2021
Merged

Switch to UUID for event IDs #926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 1 deletion cmd/triggerrun/cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func trigger(triggerFile, httpPath, action, kubeconfig string, writer io.Writer)

logger, _ := zap.NewProduction()
sugerLogger := logger.Sugar()
eventID := template.UID()
eventID := template.UUID()
r := newSink(config, sugerLogger)
eventLog := sugerLogger.With(zap.String(triggersv1.EventIDLabelKey, eventID))
for _, tri := range triggers {
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/google/cel-go v0.6.0
github.com/google/go-cmp v0.5.4
github.com/google/go-github/v31 v31.0.0
github.com/google/uuid v1.2.0
github.com/gorilla/mux v1.7.4
github.com/spf13/cobra v1.0.0
github.com/tektoncd/pipeline v0.20.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,8 @@ github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY=
github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y=
github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.2.0 h1:qJYtXnJRWmpe7m/3XlyhrsLrEURqHRM2kxzoxXqyUDs=
github.com/google/uuid v1.2.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/googleapis/gax-go v2.0.2+incompatible h1:silFMLAnr330+NRuag/VjIGF7TLp/LBrV2CJKFLWEww=
github.com/googleapis/gax-go v2.0.2+incompatible/go.mod h1:SFVmujtThgffbyetf+mdk2eWhX2bMyUtNHzFKcPA9HY=
github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
Expand Down
2 changes: 1 addition & 1 deletion pkg/sink/sink.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (r Sink) HandleEvent(response http.ResponseWriter, request *http.Request) {
return
}

eventID := template.UID()
eventID := template.UUID()
eventLog := r.Logger.With(zap.String(triggersv1.EventIDLabelKey, eventID))
eventLog.Debugf("EventListener: %s in Namespace: %s handling event (EventID: %s) with payload: %s and header: %v",
r.EventListenerName, r.EventListenerNamespace, eventID, string(event), request.Header)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sink/sink_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const (

func init() {
// Override UID generator for consistent test results.
template.UID = func() string { return eventID }
template.UUID = func() string { return eventID }
}

// getSinkAssets seeds test resources and returns a testable Sink and a dynamic client. The returned client is used to
Expand Down
2 changes: 1 addition & 1 deletion pkg/template/event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func ResolveParams(rt ResolvedTrigger, body []byte, header http.Header, extensio
// ResolveResources resolves a templated resource by replacing params with their values.
func ResolveResources(template *triggersv1.TriggerTemplate, params []triggersv1.Param) []json.RawMessage {
resources := make([]json.RawMessage, len(template.Spec.ResourceTemplates))
uid := UID()
uid := UUID()

oldEscape := metav1.HasAnnotation(template.ObjectMeta, OldEscapeAnnotation)

Expand Down
15 changes: 9 additions & 6 deletions pkg/template/event_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,19 @@ limitations under the License.
package template

import (
"bytes"
"encoding/json"
"net/http"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/google/uuid"
triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
"github.com/tektoncd/triggers/test"
bldr "github.com/tektoncd/triggers/test/builder"
"k8s.io/apimachinery/pkg/runtime"
utilrand "k8s.io/apimachinery/pkg/util/rand"
)

const (
Expand Down Expand Up @@ -532,7 +533,7 @@ func TestResolveResources(t *testing.T) {
bldr.TriggerResourceTemplate(runtime.RawExtension{Raw: []byte(`{"rt1": "$(uid)"}`)}),
)),
want: []json.RawMessage{
json.RawMessage(`{"rt1": "cbhtc"}`),
json.RawMessage(`{"rt1": "31313131-3131-4131-b131-313131313131"}`),
},
}, {
name: "uid replacement is consistent across multiple templates",
Expand All @@ -541,15 +542,17 @@ func TestResolveResources(t *testing.T) {
bldr.TriggerResourceTemplate(runtime.RawExtension{Raw: []byte(`{"rt2": "$(uid)"}`)}),
)),
want: []json.RawMessage{
json.RawMessage(`{"rt1": "cbhtc"}`),
json.RawMessage(`{"rt2": "cbhtc"}`),
json.RawMessage(`{"rt1": "31313131-3131-4131-b131-313131313131"}`),
json.RawMessage(`{"rt2": "31313131-3131-4131-b131-313131313131"}`),
},
}}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Seeded for UID() to return "cbhtc"
utilrand.Seed(0)
// Seeded for UUID() to return "31313131-3131-4131-b131-313131313131"
reader := bytes.NewReader([]byte("1111111111111111"))
uuid.SetRand(reader)
uuid.SetClockSequence(1)
got := ResolveResources(addOldEscape(tt.template), tt.params)
// Use toString so that it is easy to compare the json.RawMessage diffs
if diff := cmp.Diff(toString(tt.want), toString(got)); diff != "" {
Expand Down
7 changes: 3 additions & 4 deletions pkg/template/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ import (
"fmt"
"strings"

"github.com/google/uuid"
triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"k8s.io/apimachinery/pkg/util/rand"
)

// uidMatch determines the uid variable within the resource template
Expand Down Expand Up @@ -136,8 +135,8 @@ func applyParamToResourceTemplate(param triggersv1.Param, rt json.RawMessage, ol
return bytes.Replace(rt, []byte(paramVariable), []byte(param.Value), -1)
}

// UID generates a random string like the Kubernetes apiserver generateName metafield postfix.
var UID = func() string { return rand.String(5) }
// UUID generates a Universally Unique IDentifier following RFC 4122.
var UUID = func() string { return uuid.New().String() }
Copy link
Member Author

Choose a reason for hiding this comment

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

I have currently changed the naming everywhere from uid to uuid - I am not certain if this is really required.

uuid is more clear as to what it is.
uid is a less breaking change (and is the name k8s uses AFAIK).

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @wlynch in case you have suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

I agree - I'm leaning towards leaving things as uid based on your reasoning + so we don't have to bother with deprecation.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is done now - the change is not only very internal, renaming only the internal functions properly.


// applyUIDToResourceTemplate returns the TriggerResourceTemplate after uid replacement
// The same uid should be used per trigger to properly address resources throughout the TriggerTemplate.
Expand Down
4 changes: 2 additions & 2 deletions vendor/github.com/google/uuid/hash.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/github.com/google/uuid/sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions vendor/github.com/google/uuid/uuid.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions vendor/github.com/google/uuid/version4.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion vendor/modules.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ github.com/google/go-github/v31/github
github.com/google/go-querystring/query
# github.com/google/gofuzz v1.1.0
github.com/google/gofuzz
# github.com/google/uuid v1.1.2
# github.com/google/uuid v1.2.0
## explicit
github.com/google/uuid
# github.com/googleapis/gax-go/v2 v2.0.5
github.com/googleapis/gax-go/v2
Expand Down