feat(kuma-cp) Proxy Template modifications #883

jakubdyszkiewicz · 2020-07-03T16:38:54Z

Summary

This PR introduces long-awaited modifications to Envoy config generated by Kuma 🎉

For now implemented parts are:

Clusters
Listener
Network filter
HTTP filter

TODO

Implement VirtualHost and Route
consider adding modifications to EDS (ClusterLoadAssignment/Endpoints. I missed it in the proposal. If not in this PR, create an issue.
Docs (I think we should include a lot of examples with explanation, just like in proposal)

Documentation

kumahq/kuma-website#238

jakubdyszkiewicz · 2020-07-03T16:44:51Z

type: ProxyTemplate
name: pt-1
mesh: default
conf:
  imports:
  - default-proxy
  modifications:
  - cluster:
      operation: patch
      value: |
        connectTimeout: "10s"
  - cluster:
      operation: add
      value: |
        connectTimeout: "5s"
        loadAssignment:
          clusterName: test:cluster
          endpoints:
          - lbEndpoints:
            - endpoint:
                address:
                  socketAddress:
                    address: 192.168.0.1
                    portValue: 8080
        name: test-cluster
        type: STATIC
  - cluster:
      operation: remove
      match:
        name: kuma:envoy:admin
  - networkFilter:
      operation: remove
      match:
        direction: outbound
  - networkFilter:
      operation: addFirst
      match:
        direction: outbound
      value: |
        name: envoy.filters.network.direct_response
        typedConfig:
          '@type': type.googleapis.com/envoy.config.filter.network.direct_response.v2.Config
          response:
            inlineString: |
              HTTP/1.1 200 OK
              content-length: 13

              Hello, World!

Some cool example for testing (patch all cluster timeouts, add a new one, remove existing. Remove all network filters on outbound and add static response)

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>

…-modifications

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>

…-modifications

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>

…-modifications

nickolaev

Pretty much straight forward. I like it a lot!

nickolaev · 2020-07-16T06:40:07Z

Makefile

+	echo '// Import all Envoy packages so protobuf are registered and are ready to used in functions such as MarshalAny.' >> imports.go
+	echo '// This file is autogenerated. run "make generate/envoy-imports" to regenerate it after go-control-plane upgrade' >> imports.go
+	echo 'import (' >> imports.go
+	go list github.com/envoyproxy/go-control-plane/... | grep "github.com/envoyproxy/go-control-plane/envoy/" | awk '{printf "\t_ \"%s\"\n", $$1}' >> imports.go


Can we run into versioning issues here? Is there a way to pin what we test and support?

As far as I know it uses version from go.mod. The only problem here is that after upgrade go-control-plane we need to rerun it, because there might be new packages.

Makefile

nickolaev · 2020-07-16T06:46:13Z

api/mesh/v1alpha1/proxy_template.proto

+      // Network Filter modification
+      NetworkFilter networkFilter = 3;
+      // HTTP Filter modification
+      HttpFilter httpFilter = 4;


Is this all? I look here https://www.envoyproxy.io/docs/envoy/latest/api-v2/api, and there are more entities.

There are only a couple of fundamental Envoy resources:

Cluster - it's here

Listener - it's here

ClusterLoadAssignment - there might be a use case of adding extra endpoint, but I don't want to implement it for now. I think I'll open an issue for this and maybe someone from the community will pick this up.

RouteConfiguration - I want to add it in a separate PR

Secret - I don't think there will be a use case for this

Runtime - again, no "popular" use case for now

You can see a lot of entities that are embedded in fundamental types. We added modifications for filters because it needs a special treatment (add in proper order). Having a "framework" for modifications we can later easily add other extension points if needed.

OK that makes sense.

nickolaev · 2020-07-16T06:46:51Z

app/kuma-dp/pkg/dataplane/accesslogs/server.go

-	"github.com/Kong/kuma/pkg/core"
-	"github.com/Kong/kuma/pkg/core/runtime/component"
+	"github.com/kumahq/kuma/pkg/core"
+	"github.com/kumahq/kuma/pkg/core/runtime/component"


That is fixed in master :)

nickolaev · 2020-07-16T06:48:47Z

pkg/core/xds/resource.go

 // Resource represents a generic xDS resource with name and version.
 type Resource struct {
 	Name     string
-	Version  string
+	Origin   string


Why removing the version?

Version is useless here. We don't use it anyways, we override version anyways (see SnapshotAutoVersioner)

jakubdyszkiewicz · 2020-07-16T09:12:25Z

Docs kumahq/kuma-website#238

lobkovilya · 2020-07-16T16:14:33Z

pkg/core/resources/apis/mesh/proxytemplate_validator.go

+		}
+	case mesh_proto.OpRemove:
+	default:
+		verr.AddViolation("operation", fmt.Sprintf("invalid operation. Available operations: %q, %q, %q, %q, %q", mesh_proto.OpAddFirst, mesh_proto.OpAddLast, mesh_proto.OpAddBefore, mesh_proto.OpAddAfter, mesh_proto.OpPatch))


You missed mesh_proto.OpRemove here

lobkovilya · 2020-07-16T16:19:49Z

pkg/core/resources/apis/mesh/proxytemplate_validator.go

+		}
+	case mesh_proto.OpRemove:
+	default:
+		verr.AddViolation("operation", fmt.Sprintf("invalid operation. Available operations: %q, %q, %q, %q, %q", mesh_proto.OpAddFirst, mesh_proto.OpAddLast, mesh_proto.OpAddBefore, mesh_proto.OpAddAfter, mesh_proto.OpPatch))


lobkovilya · 2020-07-16T16:25:43Z

pkg/core/resources/apis/mesh/validators.go

+func ValidateResourceYAML(msg proto.Message, resYAML string) error {
+	json, err := yaml.YAMLToJSON([]byte(resYAML))
+	if err != nil {
+		json = []byte(resYAML)


Is it legal? If err is not nil then resYAML already json for sure?

Oh, probably you imply that it will fail further

lobkovilya · 2020-07-16T16:38:41Z

pkg/core/xds/resource.go

-	if s.typeToNamesIndex[typeName] == nil {
-		s.typeToNamesIndex[typeName] = map[string]bool{}
+	for typ, resources := range set.typeToNamesIndex {
+		if s.typeToNamesIndex[typ] == nil {


I think you can reuse s.Add here

Well, not exactly. I'd have to do s.Add(set.List()...) which creates a lot of lists etc.

lobkovilya · 2020-07-16T16:44:27Z

pkg/util/proto/any.go

+	msgTypeName := strings.ReplaceAll(dst.TypeUrl, googleApis, "") // TypeURL in Any contains type.googleapis.com/ prefix, but in Proto registry it does not have this prefix.
+	msgType := proto.MessageType(msgTypeName).Elem()
+
+	dstMsg := reflect.New(msgType).Interface().(proto.Message)


There is ptypes.UnmarshalAny(any *any.Any, pb proto.Message) error, so you don't need to use reflect

Yes but then what do you pass as pb proto.Message? We don't know what is the type.
I tried to experiment with DynamicAny

func MergeAnys(dst *any.Any, src *any.Any) (*any.Any, error) { if src == nil { return dst, nil } if dst == nil { return src, nil } if src.TypeUrl != dst.TypeUrl { return nil, errors.Errorf("type URL of dst %q is different than src %q", dst.TypeUrl, src.TypeUrl) } dstMsg := &ptypes.DynamicAny{} if err := ptypes.UnmarshalAny(dst, dstMsg); err != nil { return nil, err } srcMsg := &ptypes.DynamicAny{} if err := ptypes.UnmarshalAny(src, srcMsg); err != nil { return nil, err } proto.Merge(dstMsg, srcMsg) any, err := MarshalAnyDeterministic(dstMsg) if err != nil { return nil, err } any.TypeUrl = src.TypeUrl return any, nil }

but there is a problem with merging on DynamicAny objects

jakubdyszkiewicz added 8 commits July 2, 2020 20:45

cluster modifications

83a2bcf

network filter

5897048

add validators

f0e1278

remove version

9be4f4f

add direction

264b42f

modifications validators

8b1166f

improved validation

35f6cc1

create new resource store with function, missing imports

6c401e1

Nikolay Nikolaev added 5 commits July 12, 2020 11:23

fix(*) fix XDS testing

6d3ece3

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>

Merge remote-tracking branch 'origin/master' into feat/proxy-template…

6704b8b

…-modifications

fix(*) fix tests after merging master

930845d

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>

Merge remote-tracking branch 'origin/master' into feat/proxy-template…

7b38580

…-modifications

fix(*) fixes after merge kumahq/kuma

ad7967c

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>

jakubdyszkiewicz mentioned this pull request Jul 13, 2020

Missing support for http2 in kuma-sidecar (envoy) #889

Closed

jakubdyszkiewicz added 9 commits July 13, 2020 15:45

regenrate gui

1567126

http filter and listener modifications

ec2ccef

direction -> origin

9aa3632

cleanup

2f57344

refactor to modificators

5c76ebd

generate envoy imports

7815f19

http filter and listener validators

66df545

Merge remote-tracking branch 'origin/master' into feat/proxy-template…

8744fd2

…-modifications

http filter and listener validators positive cases

50cba24

jakubdyszkiewicz marked this pull request as ready for review July 15, 2020 11:06

jakubdyszkiewicz requested a review from a team as a code owner July 15, 2020 11:06

nickolaev reviewed Jul 16, 2020

View reviewed changes

jakubdyszkiewicz added 2 commits July 16, 2020 10:49

properly merge anys

fd77063

makefile generate envoy imports with var

8ec5e92

jakubdyszkiewicz requested a review from nickolaev July 16, 2020 09:12

fix exclusive ifs

4c2127c

lobkovilya reviewed Jul 16, 2020

View reviewed changes

review comments

a2666f2

jakubdyszkiewicz requested a review from lobkovilya July 17, 2020 08:07

lobkovilya approved these changes Jul 17, 2020

View reviewed changes

jakubdyszkiewicz merged commit 9e7fa3b into master Jul 17, 2020

jakubdyszkiewicz deleted the feat/proxy-template-modifications branch July 17, 2020 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(kuma-cp) Proxy Template modifications #883

feat(kuma-cp) Proxy Template modifications #883

jakubdyszkiewicz commented Jul 3, 2020 •

edited

jakubdyszkiewicz commented Jul 3, 2020

nickolaev left a comment

nickolaev Jul 16, 2020

jakubdyszkiewicz Jul 16, 2020

nickolaev Jul 16, 2020

jakubdyszkiewicz Jul 16, 2020

nickolaev Jul 16, 2020

nickolaev Jul 16, 2020

nickolaev Jul 16, 2020

jakubdyszkiewicz Jul 16, 2020

jakubdyszkiewicz commented Jul 16, 2020

lobkovilya Jul 16, 2020

lobkovilya Jul 16, 2020

lobkovilya Jul 16, 2020

lobkovilya Jul 16, 2020

lobkovilya Jul 16, 2020

jakubdyszkiewicz Jul 17, 2020

lobkovilya Jul 16, 2020 •

edited

jakubdyszkiewicz Jul 17, 2020

feat(kuma-cp) Proxy Template modifications #883

feat(kuma-cp) Proxy Template modifications #883

Conversation

jakubdyszkiewicz commented Jul 3, 2020 • edited

Summary

TODO

Documentation

jakubdyszkiewicz commented Jul 3, 2020

nickolaev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubdyszkiewicz commented Jul 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lobkovilya Jul 16, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubdyszkiewicz commented Jul 3, 2020 •

edited

lobkovilya Jul 16, 2020 •

edited