Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 11 additions & 1 deletion .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,17 @@ run:

linters:
enable:
- unconvert
- asciicheck
- gosec
- prealloc
- unconvert
- unparam
disable:
- errcheck

issues:
exclude-rules:
- path: test # Excludes /test, *_test.go etc.
linters:
- gosec
- unparam
6 changes: 4 additions & 2 deletions cmd/v0.16/broker-cleanup/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ func main() {
}

foundBrokerForCleaning := false
for _, broker := range brokers.Items {
for i := range brokers.Items {
broker := brokers.Items[i]
clean := false

if broker.Annotations["eventing.knative.dev/broker.class"] == "ChannelBasedBroker" {
Expand Down Expand Up @@ -232,7 +233,8 @@ func main() {
}

if !env.DryRun {
for _, b := range relabel {
for i := range relabel {
b := relabel[i]
b.Annotations["eventing.knative.dev/broker.class"] = env.ReplacementBrokerClass
if _, err := client.EventingV1beta1().Brokers(b.Namespace).Update(ctx, &b, metav1.UpdateOptions{}); err != nil {
fmt.Printf("# [error] failed to update broker class for %s/%s: %s\n", b.Namespace, b.Name, err)
Expand Down
3 changes: 2 additions & 1 deletion cmd/v0.17/pingsource-cleanup/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ func main() {
}

if !env.DryRun {
for _, ref := range cleanups {
for i := range cleanups {
ref := cleanups[i]
Comment on lines -75 to +76
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you know which linter does this? I am curious why this is better

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.

It's gosec, see https://github.com/golang/go/wiki/CommonMistakes#using-reference-to-loop-iterator-variable.

It's not "better" per se, but the reference taken here might become invalid shortly, hence it ain't a good idea.

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.

That is to say: This isn't necessary in many many cases, using the range clause is fine for most cases actually. Only if you're explicitly taking a reference to the iterator variable are you potentially going to have a bad time.

fmt.Printf("# will remove finalizer for %s/%s\n", ref.Namespace, ref.Name)

finalizers := sets.NewString(ref.Finalizers...)
Expand Down
1 change: 1 addition & 0 deletions pkg/adapter/mtping/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func (a *cronJobsRunner) cronTick(ctx context.Context, event cloudevents.Event)
event.SetID(uuid.New().String()) // provide an ID here so we can track it with logging
target := cecontext.TargetFrom(ctx).String()
source := event.Context.GetSource()
// nolint:gosec // Cryptographic randomness not necessary here.
time.Sleep(time.Duration(rand.Intn(500)) * time.Millisecond) // provide a delay so not all ping fired instantaneously distribute load on resources.

a.Logger.Debugf("sending cloudevent id: %s, source: %s, target: %s", event.ID(), source, target)
Expand Down
6 changes: 3 additions & 3 deletions pkg/adapter/v2/cloudevents.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ var _ cloudevents.Client = (*client)(nil)

// Send implements client.Send
func (c *client) Send(ctx context.Context, out event.Event) protocol.Result {
c.applyOverrides(ctx, &out)
c.applyOverrides(&out)
res := c.ceClient.Send(ctx, out)
return c.reportCount(ctx, out, res)
}

// Request implements client.Request
func (c *client) Request(ctx context.Context, out event.Event) (*event.Event, protocol.Result) {
c.applyOverrides(ctx, &out)
c.applyOverrides(&out)
resp, res := c.ceClient.Request(ctx, out)
return resp, c.reportCount(ctx, out, res)
}
Expand All @@ -95,7 +95,7 @@ func (c *client) StartReceiver(ctx context.Context, fn interface{}) error {
return errors.New("not implemented")
}

func (c *client) applyOverrides(ctx context.Context, event *cloudevents.Event) {
func (c *client) applyOverrides(event *cloudevents.Event) {
if c.ceOverrides != nil && c.ceOverrides.Extensions != nil {
for n, v := range c.ceOverrides.Extensions {
event.SetExtension(n, v)
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/duck/v1/fuzzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ var FuzzerFuncs = fuzzer.MergeFuzzerFuncs(
if ds.BackoffPolicy != nil && *ds.BackoffPolicy == "" {
ds.BackoffPolicy = nil
} else {
// nolint:gosec // Cryptographic randomness is not necessary.
ds.BackoffPolicy = bops[rand.Intn(3)]
}
},
Expand Down
12 changes: 6 additions & 6 deletions pkg/apis/duck/v1alpha1/subscribable_types_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ func (sink *SubscribableTypeSpec) ConvertFrom(ctx context.Context, obj apis.Conv
sink.Subscribable = &Subscribable{
Subscribers: make([]SubscriberSpec, len(source.Subscribers)),
}
for i, s := range source.Subscribers {
if err := sink.Subscribable.Subscribers[i].ConvertFrom(ctx, &s); err != nil {
for i := range source.Subscribers {
if err := sink.Subscribable.Subscribers[i].ConvertFrom(ctx, &source.Subscribers[i]); err != nil {
return err
}
}
Expand All @@ -202,8 +202,8 @@ func (sink *SubscribableTypeSpec) ConvertFrom(ctx context.Context, obj apis.Conv
sink.Subscribable = &Subscribable{
Subscribers: make([]SubscriberSpec, len(source.Subscribers)),
}
for i, s := range source.Subscribers {
if err := sink.Subscribable.Subscribers[i].ConvertFrom(ctx, &s); err != nil {
for i := range source.Subscribers {
if err := sink.Subscribable.Subscribers[i].ConvertFrom(ctx, &source.Subscribers[i]); err != nil {
return err
}
}
Expand Down Expand Up @@ -268,9 +268,9 @@ func (sink *SubscribableTypeStatus) ConvertFrom(ctx context.Context, obj apis.Co
sink.SubscribableStatus = &SubscribableStatus{
Subscribers: make([]eventingduckv1beta1.SubscriberStatus, len(source.Subscribers)),
}
for i, ss := range source.Subscribers {
for i := range source.Subscribers {
sink.SubscribableStatus.Subscribers[i] = eventingduckv1beta1.SubscriberStatus{}
if err := sink.SubscribableStatus.Subscribers[i].ConvertFrom(ctx, &ss); err != nil {
if err := sink.SubscribableStatus.Subscribers[i].ConvertFrom(ctx, &source.Subscribers[i]); err != nil {
return err
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/duck/v1beta1/subscribable_types_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ func (sink *SubscribableSpec) ConvertFrom(ctx context.Context, obj apis.Converti
case *eventingduckv1.SubscribableSpec:
if len(source.Subscribers) > 0 {
sink.Subscribers = make([]SubscriberSpec, len(source.Subscribers))
for i, s := range source.Subscribers {
if err := sink.Subscribers[i].ConvertFrom(ctx, &s); err != nil {
for i := range source.Subscribers {
if err := sink.Subscribers[i].ConvertFrom(ctx, &source.Subscribers[i]); err != nil {
return err
}
}
Expand Down Expand Up @@ -172,9 +172,9 @@ func (sink *SubscribableStatus) ConvertFrom(ctx context.Context, obj apis.Conver
case *eventingduckv1.SubscribableStatus:
if len(source.Subscribers) > 0 {
sink.Subscribers = make([]SubscriberStatus, len(source.Subscribers))
for i, ss := range source.Subscribers {
for i := range source.Subscribers {
sink.Subscribers[i] = SubscriberStatus{}
if err := sink.Subscribers[i].ConvertFrom(ctx, &ss); err != nil {
if err := sink.Subscribers[i].ConvertFrom(ctx, &source.Subscribers[i]); err != nil {
return err
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/sources/v1alpha2/container_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func (cs *ContainerSourceSpec) Validate(ctx context.Context) *apis.FieldError {
fe := apis.ErrMissingField("containers")
errs = errs.Also(fe)
} else {
for i, c := range cs.Template.Spec.Containers {
if ce := isValidContainer(&c); ce != nil {
for i := range cs.Template.Spec.Containers {
if ce := isValidContainer(&cs.Template.Spec.Containers[i]); ce != nil {
errs = errs.Also(ce.ViaFieldIndex("containers", i))
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/sources/v1beta1/container_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func (cs *ContainerSourceSpec) Validate(ctx context.Context) *apis.FieldError {
fe := apis.ErrMissingField("containers")
errs = errs.Also(fe)
} else {
for i, c := range cs.Template.Spec.Containers {
if ce := isValidContainer(&c); ce != nil {
for i := range cs.Template.Spec.Containers {
if ce := isValidContainer(&cs.Template.Spec.Containers[i]); ce != nil {
errs = errs.Also(ce.ViaFieldIndex("containers", i))
}
}
Expand Down
14 changes: 6 additions & 8 deletions pkg/mtbroker/filter/filter_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,12 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) {

h.reportArrivalTime(event, reportArgs)

h.send(ctx, writer, request.Header, subscriberURI.String(), reportArgs, event, ttl, span)
h.send(ctx, writer, request.Header, subscriberURI.String(), reportArgs, event, ttl)
}

func (h *Handler) send(ctx context.Context, writer http.ResponseWriter, headers http.Header, target string, reportArgs *ReportArgs, event *cloudevents.Event, ttl int32, span *trace.Span) {

func (h *Handler) send(ctx context.Context, writer http.ResponseWriter, headers http.Header, target string, reportArgs *ReportArgs, event *cloudevents.Event, ttl int32) {
// send the event to trigger's subscriber
response, err := h.sendEvent(ctx, headers, target, event, reportArgs, span)
response, err := h.sendEvent(ctx, headers, target, event, reportArgs)
if err != nil {
h.logger.Error("failed to send event", zap.Error(err))
writer.WriteHeader(http.StatusInternalServerError)
Expand All @@ -216,7 +215,7 @@ func (h *Handler) send(ctx context.Context, writer http.ResponseWriter, headers
}

// If there is an event in the response write it to the response
statusCode, err := writeResponse(ctx, writer, response, ttl, span)
statusCode, err := writeResponse(ctx, writer, response, ttl)
if err != nil {
h.logger.Error("failed to write response", zap.Error(err))
// Ok, so writeResponse will return the HttpStatus of the function. That may have
Expand All @@ -232,7 +231,7 @@ func (h *Handler) send(ctx context.Context, writer http.ResponseWriter, headers
writer.WriteHeader(statusCode)
}

func (h *Handler) sendEvent(ctx context.Context, headers http.Header, target string, event *cloudevents.Event, reporterArgs *ReportArgs, span *trace.Span) (*http.Response, error) {
func (h *Handler) sendEvent(ctx context.Context, headers http.Header, target string, event *cloudevents.Event, reporterArgs *ReportArgs) (*http.Response, error) {
// Send the event to the subscriber
req, err := h.sender.NewCloudEventRequestWithTarget(ctx, target)
if err != nil {
Expand Down Expand Up @@ -265,8 +264,7 @@ func (h *Handler) sendEvent(ctx context.Context, headers http.Header, target str
return resp, err
}

func writeResponse(ctx context.Context, writer http.ResponseWriter, resp *http.Response, ttl int32, span *trace.Span) (int, error) {

func writeResponse(ctx context.Context, writer http.ResponseWriter, resp *http.Response, ttl int32) (int, error) {
response := cehttp.NewMessageFromHttpResponse(resp)
defer response.Finish(nil)

Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/eventtype/eventtype.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ var _ eventtypereconciler.Interface = (*Reconciler)(nil)
// 2. Verify the Broker is ready.
// TODO remove https://github.com/knative/eventing/issues/2750
func (r *Reconciler) ReconcileKind(ctx context.Context, et *v1beta1.EventType) pkgreconciler.Event {
b, err := r.getBroker(ctx, et)
b, err := r.getBroker(et)
if err != nil {
if apierrs.IsNotFound(err) {
logging.FromContext(ctx).Errorw("Broker does not exist", zap.Error(err))
Expand Down Expand Up @@ -80,6 +80,6 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, et *v1beta1.EventType) p
}

// getBroker returns the Broker for EventType 'et' if it exists, otherwise it returns an error.
func (r *Reconciler) getBroker(ctx context.Context, et *v1beta1.EventType) (*v1beta1.Broker, error) {
func (r *Reconciler) getBroker(et *v1beta1.EventType) (*v1beta1.Broker, error) {
return r.brokerLister.Brokers(et.Namespace).Get(et.Spec.Broker)
}
21 changes: 9 additions & 12 deletions pkg/reconciler/inmemorychannel/controller/inmemorychannel.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"go.uber.org/zap"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/equality"
apierrs "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -184,8 +183,7 @@ func (r *Reconciler) reconcileDispatcher(ctx context.Context, scope, dispatcherN
return nil, err
}

_, err = r.reconcileRoleBinding(ctx, dispatcherName, dispatcherNamespace, imc, dispatcherName, sa)
if err != nil {
if err := r.reconcileRoleBinding(ctx, dispatcherName, dispatcherNamespace, imc, dispatcherName, sa); err != nil {
return nil, err
}

Expand All @@ -194,8 +192,7 @@ func (r *Reconciler) reconcileDispatcher(ctx context.Context, scope, dispatcherN
// subject in the dispatcher's namespace.
// TODO: might change when ConfigMapPropagation lands
roleBindingName := fmt.Sprintf("%s-%s", dispatcherName, dispatcherNamespace)
_, err = r.reconcileRoleBinding(ctx, roleBindingName, r.systemNamespace, imc, "eventing-config-reader", sa)
if err != nil {
if err := r.reconcileRoleBinding(ctx, roleBindingName, r.systemNamespace, imc, "eventing-config-reader", sa); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -250,23 +247,23 @@ func (r *Reconciler) reconcileServiceAccount(ctx context.Context, dispatcherName
return sa, nil
}

func (r *Reconciler) reconcileRoleBinding(ctx context.Context, name string, ns string, imc *v1.InMemoryChannel, clusterRoleName string, sa *corev1.ServiceAccount) (*rbacv1.RoleBinding, error) {
rb, err := r.roleBindingLister.RoleBindings(ns).Get(name)
func (r *Reconciler) reconcileRoleBinding(ctx context.Context, name string, ns string, imc *v1.InMemoryChannel, clusterRoleName string, sa *corev1.ServiceAccount) error {
_, err := r.roleBindingLister.RoleBindings(ns).Get(name)
if err != nil {
if apierrs.IsNotFound(err) {
expected := resources.MakeRoleBinding(ns, name, sa, clusterRoleName)
rb, err := r.kubeClientSet.RbacV1().RoleBindings(ns).Create(ctx, expected, metav1.CreateOptions{})
_, err := r.kubeClientSet.RbacV1().RoleBindings(ns).Create(ctx, expected, metav1.CreateOptions{})
if err != nil {
return rb, newRoleBindingWarn(err)
return newRoleBindingWarn(err)
}
controller.GetEventRecorder(ctx).Eventf(imc, corev1.EventTypeNormal, dispatcherRoleBindingCreated, "Dispatcher RoleBinding created")
return rb, nil
return nil
}
logging.FromContext(ctx).Error("Unable to get the dispatcher RoleBinding", zap.Error(err))
imc.Status.MarkDispatcherFailed("DispatcherRoleBindingGetFailed", "Failed to get dispatcher RoleBinding")
return nil, newRoleBindingWarn(err)
return newRoleBindingWarn(err)
}
return rb, nil
return nil
}

func (r *Reconciler) reconcileDispatcherService(ctx context.Context, scope, dispatcherNamespace string, imc *v1.InMemoryChannel) (*corev1.Service, error) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/mtbroker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, b *eventingv1.Broker) pk
return err
}

triggerChan, err := r.reconcileChannel(ctx, chanMan.inf, chanMan.ref, c, b)
triggerChan, err := r.reconcileChannel(ctx, chanMan.inf, chanMan.ref, c)
if err != nil {
logging.FromContext(ctx).Errorw("Problem reconciling the trigger channel", zap.Error(err))
b.Status.MarkTriggerChannelFailed("ChannelFailure", "%v", err)
Expand Down Expand Up @@ -227,7 +227,7 @@ func (r *Reconciler) getChannelTemplate(ctx context.Context, b *eventingv1.Broke
}

// reconcileChannel reconciles Broker's 'b' underlying channel.
func (r *Reconciler) reconcileChannel(ctx context.Context, channelResourceInterface dynamic.ResourceInterface, channelObjRef corev1.ObjectReference, newChannel *unstructured.Unstructured, b *eventingv1.Broker) (*duckv1.Channelable, error) {
func (r *Reconciler) reconcileChannel(ctx context.Context, channelResourceInterface dynamic.ResourceInterface, channelObjRef corev1.ObjectReference, newChannel *unstructured.Unstructured) (*duckv1.Channelable, error) {
lister, err := r.channelableTracker.ListerFor(channelObjRef)
if err != nil {
logging.FromContext(ctx).Errorw(fmt.Sprintf("Error getting lister for Channel: %s/%s", channelObjRef.Namespace, channelObjRef.Name), zap.Error(err))
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/mtbroker/trigger/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, t *eventingv1.Trigger) p
return nil
}

brokerTrigger, err := getBrokerChannelRef(ctx, b)
brokerTrigger, err := getBrokerChannelRef(b)
if err != nil {
t.Status.MarkBrokerFailed("MissingBrokerChannel", "Failed to get broker %q annotations: %s", t.Spec.Broker, err)
return fmt.Errorf("failed to find Broker's Trigger channel: %s", err)
Expand Down Expand Up @@ -278,7 +278,7 @@ func (r *Reconciler) propagateDependencyReadiness(ctx context.Context, t *eventi
return nil
}

func getBrokerChannelRef(ctx context.Context, b *eventingv1.Broker) (*corev1.ObjectReference, error) {
func getBrokerChannelRef(b *eventingv1.Broker) (*corev1.ObjectReference, error) {
if b.Status.Annotations != nil {
ref := &corev1.ObjectReference{
Kind: b.Status.Annotations[eventing.BrokerChannelKindStatusAnnotationKey],
Expand Down
19 changes: 7 additions & 12 deletions pkg/reconciler/source/crd/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, crd *v1.CustomResourceDe
// 2. Dynamically create a controller for it, if not present already. Such controller is in charge of reconciling
// duckv1.Source resources with that particular GVR..

gvr, gvk, err := r.resolveGroupVersions(ctx, crd)
gvr, gvk, err := r.resolveGroupVersions(crd)
if err != nil {
logging.FromContext(ctx).Errorw("Error while resolving GVR and GVK", zap.String("CRD", crd.Name), zap.Error(err))
return err
Expand All @@ -75,16 +75,12 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, crd *v1.CustomResourceDe
return nil
}

err = r.reconcileController(ctx, crd, gvr, gvk)
if err != nil {
logging.FromContext(ctx).Errorw("Error while reconciling controller", zap.String("GVR", gvr.String()), zap.String("GVK", gvk.String()), zap.Error(err))
return err
}
r.reconcileController(ctx, crd, gvr, gvk)

return nil
}

func (r *Reconciler) resolveGroupVersions(ctx context.Context, crd *v1.CustomResourceDefinition) (*schema.GroupVersionResource, *schema.GroupVersionKind, error) {
func (r *Reconciler) resolveGroupVersions(crd *v1.CustomResourceDefinition) (*schema.GroupVersionResource, *schema.GroupVersionKind, error) {
var gvr *schema.GroupVersionResource
var gvk *schema.GroupVersionKind
for _, v := range crd.Spec.Versions {
Expand Down Expand Up @@ -127,27 +123,27 @@ func (r *Reconciler) deleteController(ctx context.Context, gvr *schema.GroupVers
}
}

func (r *Reconciler) reconcileController(ctx context.Context, crd *v1.CustomResourceDefinition, gvr *schema.GroupVersionResource, gvk *schema.GroupVersionKind) error {
func (r *Reconciler) reconcileController(ctx context.Context, crd *v1.CustomResourceDefinition, gvr *schema.GroupVersionResource, gvk *schema.GroupVersionKind) {
r.lock.RLock()
_, found := r.controllers[*gvr]
r.lock.RUnlock()
if found {
return nil
return
}

r.lock.Lock()
defer r.lock.Unlock()
// Now that we grabbed the write lock, check that nobody has created the controller.
_, found = r.controllers[*gvr]
if found {
return nil
return
}

// Source Duck controller constructor
sdc := duck.NewController(crd.Name, *gvr, *gvk)
if sdc == nil {
logging.FromContext(ctx).Errorw("Source Duck Controller is nil.", zap.String("GVR", gvr.String()), zap.String("GVK", gvk.String()))
return nil
return
}

// Source Duck controller context
Expand All @@ -169,5 +165,4 @@ func (r *Reconciler) reconcileController(ctx context.Context, crd *v1.CustomReso
}
}
}(rc.controller)
return nil
}
Loading