Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
db8ab30
removing validations of Kind Channel and apiVersion.
nachocano May 23, 2019
fb2505f
adding subscribable type check
nachocano May 24, 2019
efcca93
more on subscription
nachocano May 25, 2019
0c133b9
Merge remote-tracking branch 'upstream/master' into subscription-channel
nachocano May 25, 2019
1358fc6
setting namespace to find the channel
nachocano May 26, 2019
ad8c164
removing pattern
nachocano May 26, 2019
0e2cf6b
Removing unnecessary changes for now.
nachocano May 28, 2019
6166c8a
Merge remote-tracking branch 'upstream/master' into subscription-channel
nachocano May 28, 2019
3abb5f4
updates
nachocano May 28, 2019
5974ce0
Merge remote-tracking branch 'upstream/master' into subscription-channel
nachocano May 28, 2019
95ff2c3
better documentation
nachocano May 28, 2019
a99bc09
Merge remote-tracking branch 'upstream/master' into subscription-channel
nachocano May 29, 2019
825bc7b
adding custom resource definition lister in subscription controller.
nachocano May 29, 2019
68227c0
Merge remote-tracking branch 'upstream/master' into subscription-channel
nachocano May 29, 2019
f40821b
remove TODO
nachocano May 29, 2019
190b448
giving permissions
nachocano May 29, 2019
f6dd4dd
adding some UTs
nachocano May 29, 2019
96283ac
updates to test
nachocano May 29, 2019
2135fd7
adding label
nachocano May 29, 2019
9383841
commenting the tracker code for now, until we have a cluster-level tr…
nachocano May 29, 2019
1684ce4
cosmetics
nachocano May 29, 2019
d48db2f
Merge remote-tracking branch 'upstream/master' into subscription-channel
nachocano May 29, 2019
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
27 changes: 27 additions & 0 deletions Gopkg.lock

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

7 changes: 7 additions & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ required = [
name = "k8s.io/apimachinery"
version = "kubernetes-1.12.6"

# Overridden to ensure compatibility with GKE
# GKE version as of 2019-01-24 is 1.11
# controller-runtime 0.1.9 requires at least 1.12
[[override]]
name = "k8s.io/apiextensions-apiserver"
version = "kubernetes-1.12.6"

# Overridden to ensure compatibility with GKE
# GKE version as of 2019-01-24 is 1.11
# controller-runtime 0.1.9 requires at least 1.12
Expand Down
8 changes: 8 additions & 0 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
pkgmetrics "github.com/knative/pkg/metrics"
"github.com/knative/pkg/signals"
"go.uber.org/zap"
apiextensionsinformers "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions"
kubeinformers "k8s.io/client-go/informers"
"k8s.io/client-go/rest"
)
Expand Down Expand Up @@ -80,6 +81,7 @@ func main() {

kubeInformerFactory := kubeinformers.NewSharedInformerFactory(opt.KubeClientSet, opt.ResyncPeriod)
eventingInformerFactory := informers.NewSharedInformerFactory(opt.EventingClientSet, opt.ResyncPeriod)
apiExtensionsInformerFactory := apiextensionsinformers.NewSharedInformerFactory(opt.ApiExtensionsClientSet, opt.ResyncPeriod)

// Eventing
triggerInformer := eventingInformerFactory.Eventing().V1alpha1().Triggers()
Expand All @@ -96,6 +98,9 @@ func main() {
serviceAccountInformer := kubeInformerFactory.Core().V1().ServiceAccounts()
roleBindingInformer := kubeInformerFactory.Rbac().V1().RoleBindings()

// Api Extensions
customResourceDefinitionInformer := apiExtensionsInformerFactory.Apiextensions().V1beta1().CustomResourceDefinitions()

// Duck
addressableInformer := duck.NewAddressableInformer(opt)

Expand All @@ -107,6 +112,7 @@ func main() {
opt,
subscriptionInformer,
addressableInformer,
customResourceDefinitionInformer,
),
namespace.NewController(
opt,
Expand Down Expand Up @@ -180,6 +186,8 @@ func main() {
deploymentInformer.Informer(),
serviceAccountInformer.Informer(),
roleBindingInformer.Informer(),
// Api Extensions
customResourceDefinitionInformer.Informer(),
); err != nil {
logger.Fatalf("Failed to start informers: %v", err)
}
Expand Down
10 changes: 10 additions & 0 deletions config/200-controller-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,13 @@ rules:
- "apiserversources/status"
- "apiserversources/finalizers"
verbs: *everything

# The subscription controller needs to retrieve and watch CustomResourceDefinitions.
- apiGroups:
- "apiextensions.k8s.io"
resources:
- "customresourcedefinitions"
verbs:
- "get"
- "list"
- "watch"
2 changes: 0 additions & 2 deletions config/300-subscription.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ spec:
minLength: 1
kind:
type: string
pattern: "^Channel$"
name:
type: string
minLength: 1
Expand Down Expand Up @@ -106,7 +105,6 @@ spec:
minLength: 1
kind:
type: string
pattern: "^Channel$"
name:
type: string
minLength: 1
3 changes: 2 additions & 1 deletion config/channels/in-memory-channel/300-in-memory-channel.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ metadata:
name: inmemorychannels.messaging.knative.dev
labels:
knative.dev/crd-install: "true"
messaging.knative.dev/subscribable: "true"
spec:
group: messaging.knative.dev
version: v1alpha1
Expand Down Expand Up @@ -88,4 +89,4 @@ spec:
minLength: 1
replyURI:
type: string
minLength: 1
minLength: 1
13 changes: 13 additions & 0 deletions pkg/apis/duck/v1alpha1/subscribable_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1alpha1

import (
"github.com/knative/pkg/apis"
"github.com/knative/pkg/apis/duck"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -34,6 +35,9 @@ type Subscribable struct {
Subscribers []ChannelSubscriberSpec `json:"subscribers,omitempty" patchStrategy:"merge" patchMergeKey:"uid"`
}

// Subscribable is an Implementable "duck type".
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should create a new Duck Type that looks something like this:

type Channelable struct {
metav1.TypeMeta ...
metav1.ObjectMeta ...
Spec ChannelSpec ...
Status Addressable ...

}

So that we have one Duck type that allows us to create Subscriptions to it as well get the Addressable part, since a Channel is both so rather than muck with addressable and subscribable, we have a proper Channelable type that handles spec / status. I think Addressable might not be enough, since we might also need some kind of Subscriptions statuses wired through. If you think this is reasonable, let's create an issue to track this, unless we think this change might be useful to do now.
@Harwayne @n3wscott

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree we need this to make things cleaner.
IMO we can track it and do it in a follow up, but waiting to hear thoughts from the guys...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fine with an issue, we need to relax this constraint to be able to use new channels, please generate an issue to track this work and I'm good.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

var _ duck.Implementable = (*Subscribable)(nil)

// ChannelSubscriberSpec defines a single subscriber to a Channel.
// Ref is a reference to the Subscription this ChannelSubscriberSpec was created for
// SubscriberURI is the endpoint for the subscriber
Expand All @@ -52,6 +56,9 @@ type ChannelSubscriberSpec struct {
ReplyURI string `json:"replyURI,omitempty"`
}

// +genclient
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// Channel is a skeleton type wrapping Subscribable in the manner we expect resource writers
// defining compatible resources to embed it. We will typically use this type to deserialize
// Channel ObjectReferences and access the Subscription data. This is not a real resource.
Expand All @@ -69,6 +76,12 @@ type ChannelSpec struct {
Subscribable *Subscribable `json:"subscribable,omitempty"`
}

var (
// Verify Channel resources meet duck contracts.
_ duck.Populatable = (*Channel)(nil)
_ apis.Listable = (*Channel)(nil)
)

// GetFullType implements duck.Implementable
func (s *Subscribable) GetFullType() duck.Populatable {
return &Channel{}
Expand Down
8 changes: 8 additions & 0 deletions pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go

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

20 changes: 2 additions & 18 deletions pkg/apis/eventing/v1alpha1/subscribable_channelable_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,9 @@ func isChannelEmpty(f corev1.ObjectReference) bool {
return equality.Semantic.DeepEqual(f, corev1.ObjectReference{})
}

// Valid from only contains the following fields:
// - Kind == 'Channel'
// - APIVersion == 'eventing.knative.dev/v1alpha1'
// - Name == not empty
// Valid if it is a valid object reference.
func isValidChannel(f corev1.ObjectReference) *apis.FieldError {
errs := isValidObjectReference(f)

if f.Kind != "Channel" {
fe := apis.ErrInvalidValue(f.Kind, "kind")
fe.Paths = []string{"kind"}
fe.Details = "only 'Channel' kind is allowed"
errs = errs.Also(fe)
}
if f.APIVersion != "eventing.knative.dev/v1alpha1" {
fe := apis.ErrInvalidValue(f.APIVersion, "apiVersion")
fe.Details = "only eventing.knative.dev/v1alpha1 is allowed for apiVersion"
errs = errs.Also(fe)
}
return errs
return isValidObjectReference(f)
}

func isValidObjectReference(f corev1.ObjectReference) *apis.FieldError {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,40 +30,22 @@ var validationTests = []struct {
want *apis.FieldError
}{
{
name: "invalid kind",
name: "valid object ref",
ref: corev1.ObjectReference{
Name: "boaty-mcboatface",
APIVersion: "eventing.knative.dev/v1alpha1",
Kind: "Strait",
},
want: &apis.FieldError{
Message: "invalid value: Strait",
Paths: []string{"kind"},
Details: "only 'Channel' kind is allowed",
},
},
{
name: "invalid api version",
ref: corev1.ObjectReference{
Name: "boaty-mcboatface",
APIVersion: "eventing.knative.dev/v1alpha2",
Kind: "Channel",
},
want: &apis.FieldError{
Message: `invalid value: eventing.knative.dev/v1alpha2`,
Paths: []string{"apiVersion"},
Details: "only eventing.knative.dev/v1alpha1 " +
"is allowed for apiVersion",
Kind: "MyChannel",
},
want: nil,
},
{
name: "valid channel",
name: "invalid object ref",
ref: corev1.ObjectReference{
Name: "boaty-mcboatface",
APIVersion: "eventing.knative.dev/v1alpha1",
Kind: "Channel",
Kind: "",
},
want: nil,
want: apis.ErrMissingField("kind"),
},
}

Expand Down
14 changes: 7 additions & 7 deletions pkg/apis/eventing/v1alpha1/subscription_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ type SubscriptionSpec struct {
// - Kind
// - APIVersion
// - Name
// Kind must be "Channel" and APIVersion must be
Comment thread
nachocano marked this conversation as resolved.
// "eventing.knative.dev/v1alpha1"
//
// The resource pointed by this ObjectReference must meet the Subscribable contract
// with a pointer to the Subscribable duck type. If the resource does not meet this contract,
// it will be reflected in the Subscription's status.

// This field is immutable. We have no good answer on what happens to
// the events that are currently in the channel being consumed from
// and what the semantics there should be. For now, you can always
Expand Down Expand Up @@ -154,14 +155,13 @@ type SubscriberSpec struct {
// ReplyStrategy specifies the handling of the SubscriberSpec's returned replies.
// If no SubscriberSpec is specified, the identity function is assumed.
type ReplyStrategy struct {
// This object must be a Channel.
//
// You can specify only the following fields of the ObjectReference:
// - Kind
// - APIVersion
// - Name
// Kind must be "Channel" and APIVersion must be
// "eventing.knative.dev/v1alpha1"
// The resource pointed by this ObjectReference must meet the Addressable contract
// with a reference to the Addressable duck type. If the resource does not meet this contract,
// it will be reflected in the Subscription's status.
// +optional
Channel *corev1.ObjectReference `json:"channel,omitempty"`
}
Expand Down
11 changes: 0 additions & 11 deletions pkg/apis/eventing/v1alpha1/subscription_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,6 @@ func isValidReply(r ReplyStrategy) *apis.FieldError {
if fe := isValidObjectReference(*r.Channel); fe != nil {
return fe.ViaField("channel")
}
if r.Channel.Kind != "Channel" {
fe := apis.ErrInvalidValue(r.Channel.Kind, "kind")
fe.Paths = []string{"kind"}
fe.Details = "only 'Channel' kind is allowed"
return fe
}
if r.Channel.APIVersion != "eventing.knative.dev/v1alpha1" {
fe := apis.ErrInvalidValue(r.Channel.APIVersion, "apiVersion")
fe.Details = "only eventing.knative.dev/v1alpha1 is allowed for apiVersion"
return fe
}
return nil
}

Expand Down
Loading