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

[1.13] Subscriptions: Fix panic when match rule is empty #7539

Merged
merged 1 commit into from Feb 15, 2024

Conversation

JoshVanL
Copy link
Contributor

Prevents a panic resulting when a subscription route rule match (spec.routes.rules.match) is empty. An empty match rule is valid and is considered "default". Error occurs because of Go interface vs implementation struct pointer nil check foot-gun.

Adds integration tests for both HTTP and gRPC subscribers.

Prevents a panic resulting when a subscription route rule match
(`spec.routes.rules.match`) is empty. An empty match rule is valid and
is considered "default". Error occurs because of Go interface vs
implementation struct pointer nil check foot-gun.

Adds integration tests for both HTTP and gRPC subscribers.

Signed-off-by: joshvanl <me@joshvanl.dev>
@JoshVanL JoshVanL requested review from a team as code owners February 15, 2024 13:09
Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e8d855b) 62.27% compared to head (c8298bc) 62.20%.

Additional details and impacted files
@@               Coverage Diff                @@
##           release-1.13    #7539      +/-   ##
================================================
- Coverage         62.27%   62.20%   -0.08%     
================================================
  Files               245      245              
  Lines             22363    22363              
================================================
- Hits              13927    13911      -16     
- Misses             7282     7296      +14     
- Partials           1154     1156       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JoshVanL JoshVanL changed the title Subscriptions: Fix panic when match rule is empty [1.13] Subscriptions: Fix panic when match rule is empty Feb 15, 2024
@JoshVanL JoshVanL mentioned this pull request Feb 15, 2024
@yaron2 yaron2 merged commit 2281058 into dapr:release-1.13 Feb 15, 2024
30 of 31 checks passed
@JoshVanL JoshVanL added this to the v1.13 milestone Feb 16, 2024
JoshVanL added a commit to JoshVanL/dapr that referenced this pull request Mar 6, 2024
Prevents a panic resulting when a subscription route rule match
(`spec.routes.rules.match`) is empty. An empty match rule is valid and
is considered "default". Error occurs because of Go interface vs
implementation struct pointer nil check foot-gun.

Adds integration tests for both HTTP and gRPC subscribers.

Signed-off-by: joshvanl <me@joshvanl.dev>
yaron2 added a commit that referenced this pull request Mar 6, 2024
* Make injector resilient to sentry unavailability (#7507)

* make injector resilient to sentry unavailability

Signed-off-by: yaron2 <schneider.yaron@live.com>

* remove redundant line

Signed-off-by: yaron2 <schneider.yaron@live.com>

---------

Signed-off-by: yaron2 <schneider.yaron@live.com>

* sentry retry up to 30s (#7508)

Signed-off-by: yaron2 <schneider.yaron@live.com>

* Update contrib to 1.13.0-rc.3 (#7509)

* update contrib to 1.13.0-rc.2

Signed-off-by: yaron2 <schneider.yaron@live.com>

* update to rc.3

Signed-off-by: yaron2 <schneider.yaron@live.com>

---------

Signed-off-by: yaron2 <schneider.yaron@live.com>

* Injector: add option to add `DAPR_HOST_IP` env var to daprd (#7511)

The `DAPR_HOST_IP` env var is used in various places in Dapr for a sidecar to know its own IP address, for example for service invocation or actor invocation.

When using the Dapr injector to add the daprd container, we can use the downstream APIs to add the `DAPR_HOST_IP` env var based on data from the controller

This option can be enabled by setting the Helm option `dapr_sidecar_injector.enableK8sDownwardAPIs=true`

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Fix state encryption regression + add integration test (#7517)

* fix state encryption regression + add intg test

Signed-off-by: yaron2 <schneider.yaron@live.com>

* linter

Signed-off-by: yaron2 <schneider.yaron@live.com>

* review feedback

Signed-off-by: yaron2 <schneider.yaron@live.com>

* linter

Signed-off-by: yaron2 <schneider.yaron@live.com>

* just use sha256, remove nolint:gosec

Signed-off-by: yaron2 <schneider.yaron@live.com>

---------

Signed-off-by: yaron2 <schneider.yaron@live.com>

* Test Integration: speed up tests 10% (#7528)

* Test Integration: speed up tests 10%

Speed up integration tests by changing poll intervals
`100*time.Millisecond` to `10*time.Millisecond`.

~4.50m to ~4.20m

Signed-off-by: joshvanl <me@joshvanl.dev>

* Wait for operator healthz before exiting to ensure no exit error

Signed-off-by: joshvanl <me@joshvanl.dev>

---------

Signed-off-by: joshvanl <me@joshvanl.dev>

* Revert selfhosted disk loader to not respect namespace (#7527)

* Revert selfhosted disk loader to not respect namespace

Revert the selfhosted component disk loader to not respect the
namespace. This will allow the selfhosted disk loader to load components
from any namespace from file.

Fixes #7523

Signed-off-by: joshvanl <me@joshvanl.dev>

* Revert component disk loader behaviour to respect component namespace
when NAMESPACE env var is set.

Signed-off-by: joshvanl <me@joshvanl.dev>

---------

Signed-off-by: joshvanl <me@joshvanl.dev>

* [1.13] Add warning that Dapr state store encryption could lead to catastrophic failures (#7524)

* [1.13] Add warning that Dapr state store encryption could lead to catastrophic failures

If the same key is used to encrypt more than 2^32 values (ie. more than 2^32 "Save" operations), it can lead to the private keys being exposed.

See: #6027
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Changed per review feedback

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

---------

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Add content-length to http channel (#7537)

* add content-length to http channel

Signed-off-by: yaron2 <schneider.yaron@live.com>

* update tests

Signed-off-by: yaron2 <schneider.yaron@live.com>

---------

Signed-off-by: yaron2 <schneider.yaron@live.com>

* Updates components-contrib to 1.13.0-rc.4 (#7538)

* Updates components-contrib to 1.13.0-rc.3

Signed-off-by: joshvanl <me@joshvanl.dev>

* mod-tidy-all

Signed-off-by: joshvanl <me@joshvanl.dev>

---------

Signed-off-by: joshvanl <me@joshvanl.dev>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>

* Subscriptions: Fix panic when match rule is empty (#7539)

Prevents a panic resulting when a subscription route rule match
(`spec.routes.rules.match`) is empty. An empty match rule is valid and
is considered "default". Error occurs because of Go interface vs
implementation struct pointer nil check foot-gun.

Adds integration tests for both HTTP and gRPC subscribers.

Signed-off-by: joshvanl <me@joshvanl.dev>

* Hot Reloading: don't watch files if not enabled (#7521)

* Hot Reloading: don't watch files if not enabled

Update hot reloading so that we don't setup file watchers on the
component directory if hot reloading is not enabled.

Signed-off-by: joshvanl <me@joshvanl.dev>

* Remove `Close` paradigm from hot reloader

Signed-off-by: joshvanl <me@joshvanl.dev>

* Linting

Signed-off-by: joshvanl <me@joshvanl.dev>

---------

Signed-off-by: joshvanl <me@joshvanl.dev>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>

* Remove pubsub content-length test which has been removed from (#7550)

release-1.13

Signed-off-by: joshvanl <me@joshvanl.dev>

* Revert ApiLevel controlling vnodes back to context metadata (#7547)

* Revert ApiLevel controlling vnodes back to context metadata

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Lint

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Fixes after review

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Set back api level to 10, just for completeness purposes

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Don’t specify APILevelSpecify api level

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Apply suggestions from code review

Co-authored-by: Josh van Leeuwen <me@joshvanl.dev>
Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com>

* Cleanup after review

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Small refactor

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Refactor multiple parameters into a request object for placement table dissemination

Signed-off-by: Elena Kolevska <elena@kolevska.com>

---------

Signed-off-by: Elena Kolevska <elena@kolevska.com>
Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com>
Co-authored-by: Josh van Leeuwen <me@joshvanl.dev>
Co-authored-by: Artur Souza <asouza.pro@gmail.com>

* Actor Reminders: Default JSON serialization. (#7548)

* Actor Reminders: Default JSON serialization.

To support downgrades to 1.12 from 1.13, this PR changes the reminder
serialization storage format back to JSON by default. This means a 1.12
actor reminder client can read reminders written by 1.13 actors.

1.13 will continue to understand both JSON and protobuf. Protobuf
serialization can be enabled with the `ActorReminderStorageProtobuf`
feature gate. The actor "API Level" has been changed back to 10.

Adds test to ensure the default serialization is JSON.

Signed-off-by: joshvanl <me@joshvanl.dev>

* Remove ActorReminderStorageProtobuf feature gate in favour of using API
level

Signed-off-by: joshvanl <me@joshvanl.dev>

* Fix api level tests

Signed-off-by: joshvanl <me@joshvanl.dev>

---------

Signed-off-by: joshvanl <me@joshvanl.dev>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>

* Update contrib to 1.13.0-rc.6 (#7553)

* update contrib to 1.13.0-rc.5

Signed-off-by: yaron2 <schneider.yaron@live.com>

* update to rc.6

Signed-off-by: yaron2 <schneider.yaron@live.com>

---------

Signed-off-by: yaron2 <schneider.yaron@live.com>

* Updates components-contrib to 1.13.0-rc.7 (#7562)

Signed-off-by: joshvanl <me@joshvanl.dev>

* update contrib to 1.13.0-rc.8 (#7567)

* Add metadata in binding response even in case of error (#7572)

* Add binding metadata even in case of error.

Signed-off-by: Artur Souza <asouza.pro@gmail.com>

* ADD IT.

Signed-off-by: Artur Souza <asouza.pro@gmail.com>

* Fix lint.

Signed-off-by: Artur Souza <asouza.pro@gmail.com>

---------

Signed-off-by: Artur Souza <asouza.pro@gmail.com>

* [Release-1.13] Upgrade to contrib `v1.13.0-rc.10` (#7577)

* Upgrade to contrib 1.13.0-rc.9

Signed-off-by: Bernd Verst <github@bernd.dev>

* Use contrib 1.13.0-rc.10 for latest kafka sarama patch version

Signed-off-by: Bernd Verst <github@bernd.dev>

---------

Signed-off-by: Bernd Verst <github@bernd.dev>

* Fix for issue 7576 (#7581) (#7587)

Signed-off-by: Guido Spadotto <guido.spadotto@profesia.it>
Co-authored-by: Guido Spadotto <guido.spad8@gmail.com>
Co-authored-by: Guido Spadotto <guido.spadotto@profesia.it>

* Adds v1.13.0 release notes (#7586)

* Adds v1.13.0 release notes

Adds docs/release_notes/v1.13.0.md

Signed-off-by: joshvanl <me@joshvanl.dev>

* Update docs/release_notes/v1.13.0.md

Co-authored-by: Paul Yuknewicz <paulyuk@microsoft.com>
Signed-off-by: Josh van Leeuwen <me@joshvanl.dev>

* Move Actor Reminder Performance higher in notes

Signed-off-by: joshvanl <me@joshvanl.dev>

* Update v1.13.0.md

Signed-off-by: Artur Souza <asouza.pro@gmail.com>

---------

Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: Josh van Leeuwen <me@joshvanl.dev>
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Co-authored-by: Paul Yuknewicz <paulyuk@microsoft.com>
Co-authored-by: Artur Souza <asouza.pro@gmail.com>

* chore: bump go to 1.21.8 and protobuf lib to 1.33.0 (#7591)

* ci: force go1.21.8 in workflows instead of go.mod

Signed-off-by: mikeee <hey@mike.ee>

* chore: bump google.golang.org/protobuf to 1.33.0

Signed-off-by: mikeee <hey@mike.ee>

* make modtidy-all

Signed-off-by: Artur Souza <asouza.pro@gmail.com>

---------

Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Co-authored-by: Artur Souza <asouza.pro@gmail.com>

---------

Signed-off-by: yaron2 <schneider.yaron@live.com>
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: Elena Kolevska <elena@kolevska.com>
Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Signed-off-by: Bernd Verst <github@bernd.dev>
Signed-off-by: Guido Spadotto <guido.spadotto@profesia.it>
Signed-off-by: Josh van Leeuwen <me@joshvanl.dev>
Signed-off-by: mikeee <hey@mike.ee>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>
Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>
Co-authored-by: Elena Kolevska <elena-kolevska@users.noreply.github.com>
Co-authored-by: Artur Souza <asouza.pro@gmail.com>
Co-authored-by: Bernd Verst <github@bernd.dev>
Co-authored-by: Guido Spadotto <guido.spad8@gmail.com>
Co-authored-by: Guido Spadotto <guido.spadotto@profesia.it>
Co-authored-by: Paul Yuknewicz <paulyuk@microsoft.com>
Co-authored-by: Mike Nguyen <hey@mike.ee>
JoshVanL added a commit to JoshVanL/dapr that referenced this pull request Mar 6, 2024
Prevents a panic resulting when a subscription route rule match
(`spec.routes.rules.match`) is empty. An empty match rule is valid and
is considered "default". Error occurs because of Go interface vs
implementation struct pointer nil check foot-gun.

Adds integration tests for both HTTP and gRPC subscribers.

Signed-off-by: joshvanl <me@joshvanl.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants