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

Feat(virtual-outbound): Add entity for virtual-outbound #2576

Merged
merged 6 commits into from
Aug 23, 2021

Conversation

lahabana
Copy link
Contributor

@lahabana lahabana commented Aug 16, 2021

Summary

Virtual-outbounds enable users to define arbitrary hostnames.

It enables features like:

  • Pod routing for stateful services
  • Transition to Kuma without changing host names.

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.

This relies on changes not backported already.

@lahabana lahabana requested a review from a team as a code owner August 16, 2021 13:58
@lahabana
Copy link
Contributor Author

Do you want me to merge this without the implem behind it to make things more reviewable?

This only registers the new policy everywhere I'll then be able to connect it and add evaluation in a follow up PR.

It currently doesn't do anything

Signed-off-by: Charly Molter <charly.molter@konghq.com>
@lahabana lahabana force-pushed the feat/virtual-outbound-policy branch 2 times, most recently from 3afa724 to 92071da Compare August 18, 2021 12:40
@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2021

Codecov Report

Merging #2576 (14084db) into master (12e56b9) will decrease coverage by 0.23%.
The diff coverage is 34.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2576      +/-   ##
==========================================
- Coverage   52.28%   52.05%   -0.24%     
==========================================
  Files         869      877       +8     
  Lines       48711    49454     +743     
==========================================
+ Hits        25470    25744     +274     
- Misses      21193    21644     +451     
- Partials     2048     2066      +18     
Impacted Files Coverage Δ
api/mesh/v1alpha1/dataplane_helpers.go 80.91% <ø> (+0.66%) ⬆️
...ns/runtime/k8s/controllers/configmap_controller.go 9.84% <0.00%> (-1.27%) ⬇️
test/e2e/virtualoutbound/virtualoutbound_k8s.go 0.00% <0.00%> (ø)
...t/e2e/virtualoutbound/virtualoutbound_universal.go 0.00% <0.00%> (ø)
...est/framework/deployments/testserver/deployment.go 0.00% <0.00%> (ø)
...est/framework/deployments/testserver/kubernetes.go 0.00% <0.00%> (ø)
test/framework/interface.go 0.00% <0.00%> (ø)
test/server/cmd/echo.go 0.00% <0.00%> (ø)
test/framework/setup.go 1.17% <13.33%> (+1.17%) ⬆️
api/mesh/v1alpha1/virtual_outbound.pb.go 34.72% <34.72%> (ø)
... and 33 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 12e56b9...14084db. Read the comment docs.

Signed-off-by: Charly Molter <charly.molter@konghq.com>
@lahabana lahabana force-pushed the feat/virtual-outbound-policy branch from 92071da to 0d04d7e Compare August 18, 2021 16:00
Signed-off-by: Charly Molter <charly.molter@konghq.com>
Copy link
Contributor

@bartsmykla bartsmykla left a comment

Choose a reason for hiding this comment

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

lgtm

pkg/core/resources/model/resource.go Show resolved Hide resolved
api/mesh/v1alpha1/virtual_outbound.proto Outdated Show resolved Hide resolved
pkg/dns/virtual_outbound_selector.go Outdated Show resolved Hide resolved
pkg/core/resources/apis/mesh/virtual_outbound_validator.go Outdated Show resolved Hide resolved
api/mesh/v1alpha1/dataplane_helpers.go Outdated Show resolved Hide resolved
mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
)

func tagKeyOrName(parameter *mesh_proto.VirtualOutbound_Conf_TemplateParameter) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand it correctly we require to add a parameter entry even if a tag has the same key as a parameter in gotemplate?
So if we have template:

host: {{ .foo }}.{{ .bar }}.{{ .baz }}

and we have DPP with inbound tags:

tags:
  foo: value1
  bar: value2
  baz: value3

Anyway I have to provide parameters section:

host: {{ .foo }}.{{ .bar }}.{{ .baz }}
parameters:
  - name: foo
  - name: bar
  - name: baz

Is it possible to resolve a template without explicit definitions of the parameters if no mapping required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what I wanted at first. However, the set of parameters is what enables me to build the right tags for the outbound on envoy.
I'm not sure it's feasible to extract the keys of the gotemplate but I can have a look.

Signed-off-by: Charly Molter <charly.molter@konghq.com>
@lahabana lahabana force-pushed the feat/virtual-outbound-policy branch from 613c230 to 48ef66d Compare August 20, 2021 14:54
…ma into feat/virtual-outbound-policy

Signed-off-by: Charly Molter <charly.molter@konghq.com>
@lahabana lahabana force-pushed the feat/virtual-outbound-policy branch from 48ef66d to 14084db Compare August 20, 2021 17:02
@lahabana lahabana merged commit 44356e4 into kumahq:master Aug 23, 2021
@lahabana lahabana deleted the feat/virtual-outbound-policy branch March 29, 2024 12:41
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

4 participants