Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Commit

Permalink
[release-0.19] Upgrade trigger's serving subscriber version to v1 (#1960
Browse files Browse the repository at this point in the history
)

* Upgrade trigger's serving subscriber version to v1

This is to handle an issue related to setting the ApiVersion of the
trigger's subscriber (knative service) to an unsupported version

* update PR format check version
  • Loading branch information
MohamedElhawaty authored Dec 2, 2020
1 parent fec41c5 commit eae120d
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/check-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2.3.1
- uses: actions/setup-go@v2.1.0
- uses: actions/setup-go@v2.1.3
with:
go-version: 1.14.4
- name: gofmt
Expand Down
3 changes: 2 additions & 1 deletion cmd/webhook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ import (

var types = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{
// For group eventing.knative.dev.
brokerv1beta1.SchemeGroupVersion.WithKind("Broker"): &brokerv1beta1.Broker{},
brokerv1beta1.SchemeGroupVersion.WithKind("Broker"): &brokerv1beta1.Broker{},
brokerv1beta1.SchemeGroupVersion.WithKind("Trigger"): &brokerv1beta1.Trigger{},

// For group messaging.cloud.google.com.
messagingv1beta1.SchemeGroupVersion.WithKind("Channel"): &messagingv1beta1.Channel{},
Expand Down
12 changes: 10 additions & 2 deletions pkg/apis/broker/v1beta1/trigger_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,16 @@ import (
"context"
)

const (
knativeServingV1ApiVersion = "serving.knative.dev/v1"
knativeServingV1Alpha1ApiVersion = "serving.knative.dev/v1alpha1"
)

// SetDefaults sets the default field values for a Trigger.
func (t *Trigger) SetDefaults(ctx context.Context) {
// The Google Cloud Broker doesn't have any custom defaults. The
// eventing webhook will add the usual defaults.
// This upgrades the `ApiVersion` of the knative serving subscriber form v1alpha1 to v1, this is intended to
// help users who are lagging in their transition to knative serving v1.
if t.Spec.Subscriber.Ref != nil && t.Spec.Subscriber.Ref.APIVersion == knativeServingV1Alpha1ApiVersion {
t.Spec.Subscriber.Ref.APIVersion = knativeServingV1ApiVersion
}
}
60 changes: 57 additions & 3 deletions pkg/apis/broker/v1beta1/trigger_defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,63 @@ package v1beta1
import (
"context"
"testing"

"github.com/google/go-cmp/cmp"
eventingv1beta1 "knative.dev/eventing/pkg/apis/eventing/v1beta1"
duckv1 "knative.dev/pkg/apis/duck/v1"
)

func TestTrigger_SetDefaults(t *testing.T) {
trig := Trigger{}
trig.SetDefaults(context.TODO())
func TestTriggerDefaults(t *testing.T) {
testCases := map[string]struct {
initial Trigger
expected Trigger
}{
"non-knative v1alpha1 service subscriber": {
initial: Trigger{
Spec: eventingv1beta1.TriggerSpec{
Subscriber: duckv1.Destination{
Ref: &duckv1.KReference{
APIVersion: "serving.knative.dev/v1",
Kind: "Service",
},
},
}},
expected: Trigger{
Spec: eventingv1beta1.TriggerSpec{
Subscriber: duckv1.Destination{
Ref: &duckv1.KReference{
APIVersion: "serving.knative.dev/v1",
Kind: "Service",
},
}}},
},
"knative v1alpha1 service subscriber": {
initial: Trigger{
Spec: eventingv1beta1.TriggerSpec{
Subscriber: duckv1.Destination{
Ref: &duckv1.KReference{
APIVersion: "serving.knative.dev/v1alpha1",
Kind: "Service",
},
},
}},
expected: Trigger{
Spec: eventingv1beta1.TriggerSpec{
Subscriber: duckv1.Destination{
Ref: &duckv1.KReference{
APIVersion: "serving.knative.dev/v1",
Kind: "Service",
},
}}},
},
}

for n, tc := range testCases {
t.Run(n, func(t *testing.T) {
tc.initial.SetDefaults(context.Background())
if diff := cmp.Diff(tc.expected, tc.initial); diff != "" {
t.Fatal("Unexpected defaults (-want, +got):", diff)
}
})
}
}

0 comments on commit eae120d

Please sign in to comment.