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

fix(kuma-cp) fault injection matching #2757

Merged
merged 4 commits into from
Sep 20, 2021
Merged

Conversation

lobkovilya
Copy link
Contributor

@lobkovilya lobkovilya commented Sep 9, 2021

Summary

Today if you apply 2 Fault Injections:

type: FaultInjection
mesh: default
name: fi1
sources:
   - match:
       kuma.io/service: demo-client-1
destinations:
   - match:
       kuma.io/service: test-server
       kuma.io/protocol: http
conf:
   abort:
     httpStatus: 401
     percentage: 100
---
type: FaultInjection
mesh: default
name: fi2
sources:
   - match:
       kuma.io/service: demo-client-2
destinations:
   - match:
       kuma.io/service: test-server
       kuma.io/protocol: http
conf:
   abort:
     httpStatus: 402
     percentage: 100

Only the latest one will be applied and replace another one.

Full changelog

  • Select all matched policies for the inbound and apply them all. We can do that because we rely on the HttpFault header matching

Issues resolved

N/A

Documentation

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

Backwards compatibility

  • Add backport-to-stable label if the code is backwards compatible. Otherwise, list breaking changes.

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya lobkovilya requested a review from a team as a code owner September 9, 2021 22:46
@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2021

Codecov Report

Merging #2757 (0a77b1f) into master (6f2b621) will increase coverage by 0.01%.
The diff coverage is 25.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2757      +/-   ##
==========================================
+ Coverage   51.93%   51.94%   +0.01%     
==========================================
  Files         877      878       +1     
  Lines       51141    51219      +78     
==========================================
+ Hits        26561    26607      +46     
- Misses      22472    22494      +22     
- Partials     2108     2118      +10     
Impacted Files Coverage Δ
pkg/core/xds/types.go 89.65% <ø> (ø)
test/e2e/matching/matching_universal.go 0.00% <0.00%> (ø)
pkg/xds/generator/inbound_proxy_generator.go 74.77% <50.00%> (ø)
...s/envoy/listeners/v3/fault_injection_configurer.go 71.01% <83.33%> (+0.86%) ⬆️
pkg/core/faultinjections/matcher.go 78.94% <100.00%> (+1.16%) ⬆️
...kg/xds/envoy/listeners/filter_chain_configurers.go 97.29% <100.00%> (ø)
pkg/xds/generator/direct_access_proxy_generator.go 82.75% <0.00%> (-1.15%) ⬇️
...kg/core/resources/apis/mesh/generated_resources.go 78.56% <0.00%> (+0.20%) ⬆️
api/observability/v1/mads.pb.go 35.56% <0.00%> (+1.03%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f2b621...0a77b1f. Read the comment docs.

Copy link
Contributor

@lahabana lahabana left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

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

This approach is great.

I'm a bit concerned about the performance impact of running a bunch of complex regexes on every request, but that's not new in this PR.

pkg/core/xds/types.go Show resolved Hide resolved
pkg/xds/envoy/listeners/filter_chain_configurers.go Outdated Show resolved Hide resolved
test/e2e/matching/matching_universal_test.go Outdated Show resolved Hide resolved
@lahabana
Copy link
Contributor

Remember to update the docs :)

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@jakubdyszkiewicz
Copy link
Contributor

Backwards compatibility
[ ] Add backport-to-stable label if the code is backwards compatible. Otherwise, list breaking changes.

please put a comment whether this meant to be backported or not

@lobkovilya
Copy link
Contributor Author

@lahabana that was kind of already declared by docs, but I replaced old-formated yaml with tabs kumahq/kuma-website#535

@lobkovilya lobkovilya merged commit 1f3b393 into master Sep 20, 2021
@lobkovilya lobkovilya deleted the fix/fi-source-matching branch September 20, 2021 07:33
mergify bot pushed a commit that referenced this pull request Sep 20, 2021
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
(cherry picked from commit 1f3b393)
nikita15p pushed a commit to nikita15p/kuma that referenced this pull request Sep 28, 2021
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
nikita15p pushed a commit to nikita15p/kuma that referenced this pull request Sep 28, 2021
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
lobkovilya added a commit that referenced this pull request Oct 4, 2021
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
(cherry picked from commit 1f3b393)

Co-authored-by: Ilya Lobkov <ilya.lobkov@konghq.com>
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.

5 participants