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.

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

kn trigger create fails without --filter flag #744

Closed
dsimansk opened this issue Mar 17, 2020 · 2 comments · Fixed by #745
Closed

kn trigger create fails without --filter flag #744

dsimansk opened this issue Mar 17, 2020 · 2 comments · Fixed by #745
Assignees
Labels
good first issue Denotes an issue ready for a new contributor. kind/bug Categorizes issue or PR as related to a bug.

Comments

@dsimansk
Copy link
Contributor

dsimansk commented Mar 17, 2020

Bug report

There was a bug in Eventing 0.13.0 that rejected triggers without filters. However, even on version 0.13.1 the creation of such trigger will fail. There's a bug in our current impl that will initialize empty trigger.Spec.Filter.Attributes regardless of no flag provided that causes the error behaviour.

Beware, that kn trigger describe has to be fixed for empty filters also.

$ kn trigger create test --broker default --sink svc:svc1
cannot create trigger 'test' in namespace 'default' because: admission webhook "validation.webhook.eventing.knative.dev" denied the request: validation failed: At least one filtered attribute must be specified: spec.filter.attributes

Expected behavior

Trigger can be created without --filter flag.

Steps to reproduce the problem

  1. kn trigger create test --broker default --sink svc:svc1

kn version

HEAD

Knative (serving/eventing) version

0.13.1

/kind good-first-issue

@dsimansk dsimansk added the kind/bug Categorizes issue or PR as related to a bug. label Mar 17, 2020
@knative-prow-robot knative-prow-robot added the good first issue Denotes an issue ready for a new contributor. label Mar 17, 2020
@mvinkler
Copy link
Contributor

Also, kn trigger create -h should be updated accordingly, now it suggests --filter is mandatory:

[root@ocp-dynamic-7202 eventing]# kn trigger create -h
Create a trigger

Usage:
  kn trigger create NAME --broker BROKER --filter KEY=VALUE --sink SINK [flags]

@dsimansk
Copy link
Contributor Author

/assign dsimansk

coryrc pushed a commit to coryrc/client that referenced this issue May 14, 2020
The two missing quotes cause the output from kubectl to being read in one go (rather than line by line). The container check logic therefore only checks the very first pod's containers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants