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

Allow users to configure xDS resources #24

Closed
6 tasks done
arkodg opened this issue Apr 28, 2022 · 16 comments
Closed
6 tasks done

Allow users to configure xDS resources #24

arkodg opened this issue Apr 28, 2022 · 16 comments
Assignees
Labels
area/api API-related issues area/extensions area/translator Issues related to Gateway's translation service, e.g. translating Gateway APIs into the IR. area/xds-server Issues related to the xDS Server used for managing Envoy configuration. documentation Improvements or additions to documentation kind/decision A record of a decision made by the community. kind/enhancement New feature or request no stalebot
Milestone

Comments

@arkodg
Copy link
Contributor

arkodg commented Apr 28, 2022

Users might want to directly configure xDS Resources in case Envoy Gateway's API doesn't support their use case.
They might want to add/merge/remove/replace xDS resources that are pushed by Envoy Gateway into Envoy Proxy.
Projects such as Istio provide APIs such as EnvoyFilter to fulfill these use cases.

Outlining work items

@arkodg
Copy link
Contributor Author

arkodg commented Apr 28, 2022

Here's one way to do it

  • Lets try and implement Global Ratelimiting directly using xDS

  • Lets define a new CRD called EnvoyPatch which can be used to represent the xDS resource to be patched.


// Defines a generic resource patch
message Patch {
  // Define the envoy configuration change as a JSONPatch
  // defined in https://datatracker.ietf.org/doc/html/rfc6902.
  // $hide_from_docs
  message JsonPatch {
    // Operations that can be performed for the patch.
    enum Operation {
      INVALID = 0;
      // If the path specifies an array index, the value
      // is inserted at the specified index.
      // If it specifies an object member that does not
      // already exist, a new member is added.
      // If the path specifies a member that does exist,
      // the value replaces the existing member value.
      ADD = 1;
      // Removes the value from the location specified in
      // the path.
      REMOVE = 2;
      // Replaces the value in the path location with the new value.
      REPLACE = 3;
    }
    // Patch operation
    Operation op = 1;
    // JSON Pointer for identifying the value based on
    // https://datatracker.ietf.org/doc/html/rfc6901.
    string path = 2;
    //  Resource patch.
    google.protobuf.Struct value = 3;
  }
  // Defines a partial envoy configuration message that can be patched
  // into the corresponding resource.
  message ProtoPatch {
    // Operations that can be performed for the patch.
    enum Operation {
      INVALID = 0;
      // Patch using proto merge semantics.
      MERGE = 1;
      // Add/Append the patch as a resource to its corresponding resource list.
      ADD = 2;
    }
    Operation op = 1;
    // Resource patch.
    google.protobuf.Struct value = 2;
  }
  oneof patch_type {
    // Patch using proto semantics.
    ProtoPatch proto_patch = 1;
  }
}

message  EnvoyPatch {
  oneof resource_type {
      // Apply envoy listener patches to the listener resource
     Patch listener = 1;
     // Apply envoy listener patches to the http_connection_manager resource
     Patch http_connection_manager = 2;
     // Apply envoy listener patches to the virtual_host resource
     Patch virtual_host = 3;
     // Apply envoy route patches to the route resource
     Patch route = 4;
     // Apply envoy cluster patches to the cluster resource
     Patch cluster = 5;
    
  } 
}

  • Lets use Custom Filters in HTTPRoute to patch VirtualHost & Cluster resources. The major assumption here is that there is a 1:1 relationship/translation between HTTPRoute & Route as well as HTTPRoute & Cluster. This ensure consistent merging outcomes.

  • One side effect here is that the Envoypatch tied to the HTTPConnectionManager or Listener will impact other HTTPRoutes attached to the same Gateway (that contains the listener fields) .

  • User Intent using Gateway API

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: Gateway
metadata:
  name: eg-gw
spec:
  gatewayClassName: eg-gc
  listeners:
    - name: example
      protocol: HTTPS
      port: 443
      hostname: example.com
      tls:
        certificateRefs:
          - kind: Secret
            group: ""
            name: example-cert
---
kind: HTTPRoute
apiVersion: gateway.networking.k8s.io/v1alpha2
metadata:
  name: example
spec:
  parentRefs:
    - name: eg-gw
  hostnames:
    - "example.com"
  rules:
    - filters:
        - type: ExtensionRef
           extensionRef:
            - group: config.eg.io
               kind: EnvoyPatch
               name: rlConnMgrPatch      
        - type: ExtensionRef
           extensionRef:
            - group: config.eg.io
               kind: EnvoyPatch
               name: rlVirtualHostPatch   
        - type: ExtensionRef
           extensionRef:
            - group: config.eg.io
               kind: EnvoyPatch
               name: rlClusterPatch           
      backendRefs:
        - name: example
           port: 8080
---
apiVersion: config.eag.io/v1alpha1
kind: EnvoyPatch
metadata:
  name: rlClusterPatch
spec:
  cluster:
    protoPatch:
      op: ADD
      value:
      name: rate_limit_cluster
      type: STRICT_DNS
      connect_timeout: 10s
       lb_policy: ROUND_ROBIN
        http2_protocol_options: {}
        load_assignment:
          cluster_name: rate_limit_cluster
          endpoints:
           - lb_endpoints:
              - endpoint:
                   address:
                     socket_address:
                       address: ratelimit.svc.cluster.local
                       port_value: 8081
---
apiVersion: config.eag.io/v1alpha1
kind: EnvoyPatch
metadata:
  name: rlVirtualHostPatch
spec:                       
  virtualHost:
    protoPatch:
      op: MERGE
        value:
           rate_limits:
             - actions:
                - remote_address: {}                       
---
apiVersion: config.eag.io/v1alpha1
kind: EnvoyPatch
metadata:
  name: rlConnMgrPatch
spec:                       
   httpConnectionManager:
       protoPatch:
         op: MERGE
         value:
           http_filters:
           - name: "envoy.filters.http.ratelimit"
             typed_config:
               "@type": "type.googleapis.com/envoy.extensions.filters.http.ratelimit.v3.RateLimit"
               domain: "eag-ratelimit"
               failure_mode_deny: true
               timeout: 1s
               rate_limit_service:
                 grpc_service:
                   envoy_grpc:
                     cluster_name: rate_limit_cluster
                 transport_api_version: V3

@danehans
Copy link
Contributor

A key point from the above EnvoyFilter reference:

This feature must be used with care, as incorrect configurations could potentially destabilize the entire mesh.

I believe @LukeShu shared concerns about exposing xDS config through the API.

I can see the need to support this but IMHO it's a long-term goal since the project has a ton of work to implement the initial system design and support the core/extended Gateway APIs.

@arkodg
Copy link
Contributor Author

arkodg commented Apr 28, 2022

sure, this might not be part of the first release but since this is highlighted in the goals, im kickstarting the discussions, since it might require upstream API changes.

@youngnick
Copy link
Contributor

Yes, one of the reasons I've been very reluctant to do anything like this for Contour in the past is because the HTTPConnectionManager filter chains have to be constructed carefully to get the outcomes we want; I feel like it's a bit too easy to create a config that has a bad interaction with a designed behavior.

@mattklein123
Copy link
Member

IMO I would avoid any kind of patch semantics at all. I think it's too confusing and error prone. The way I would model this is:

  1. You are either using the official API (no exceptions)
  2. You are in xDS mode and you are on your own, and we will offer a conversion that will spit out (1) -> (2) to get you started.

IMO this is the best path forward.

@arkodg
Copy link
Contributor Author

arkodg commented Apr 29, 2022

cool, sg, should we start off with the same approach - all official EG or all BYO, for Bootstrap as well ?

@danehans
Copy link
Contributor

cool, sg, should we start off with the same approach - all official EG or all BYO, for Bootstrap as well ?

+1 for consistency

@danehans danehans added the kind/question Further information is requested label Apr 29, 2022
@danehans danehans modified the milestone: v0 Release Apr 29, 2022
@youngnick
Copy link
Contributor

I definitely agree with @mattklein123 - part of the point of the upstream API is to have extension points to allow users to model concepts that we may use xDS to implement - there are a number of extension points and the Policy APIs for this exact reason.

@danehans danehans changed the title [xDS] Allow users to configure xDS resources Allow users to configure xDS resources May 11, 2022
@danehans danehans added area/api API-related issues area/xds-server Issues related to the xDS Server used for managing Envoy configuration. documentation Improvements or additions to documentation kind/enhancement New feature or request labels May 11, 2022
@github-actions
Copy link

github-actions bot commented Jul 1, 2022

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jul 1, 2022
@github-actions
Copy link

github-actions bot commented Jul 8, 2022

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@github-actions github-actions bot closed this as completed Jul 8, 2022
@arkodg arkodg removed the stale label Aug 18, 2022
@arkodg arkodg added this to the Backlog milestone Aug 18, 2022
@arkodg arkodg reopened this Aug 18, 2022
@NomadXD
Copy link
Contributor

NomadXD commented Aug 19, 2022

@youngnick What Policy APIs you are referring here ? Kubernetes Gateway API policy attachment ? https://gateway-api.sigs.k8s.io/references/policy-attachment/

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Sep 18, 2022
@danehans danehans added the help wanted Extra attention is needed label Sep 18, 2022
@github-actions github-actions bot removed the stale label Sep 19, 2022
arkodg added a commit to arkodg/gateway that referenced this issue May 9, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit to arkodg/gateway that referenced this issue May 9, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit to arkodg/gateway that referenced this issue May 31, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit to arkodg/gateway that referenced this issue May 31, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
zirain pushed a commit that referenced this issue Jun 6, 2023
* feat: EnvoyPatchPolicy API

Relates to #24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* wip design doc

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* wrap up design

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* update implementation

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* address comments

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* charts

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Co-authored-by: Xunzhuo <bitliu@tencent.com>
arkodg added a commit to arkodg/gateway that referenced this issue Jun 29, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
zirain added a commit that referenced this issue Jul 5, 2023
* Implement JSON Patch in Xds Translator

Relates to #24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* use temp variable to unmarshal into

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* fix test

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* use apiextensionsv1.JSON

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* routeConfig test

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* add entire resource and more test cases

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* move marshaller out of for loop

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* address comments

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Co-authored-by: zirain <zirain2009@gmail.com>
arkodg added a commit to arkodg/gateway that referenced this issue Jul 5, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit to arkodg/gateway that referenced this issue Jul 5, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit to arkodg/gateway that referenced this issue Jul 6, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
zirain pushed a commit that referenced this issue Jul 8, 2023
Relates to #24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit to arkodg/gateway that referenced this issue Jul 20, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit to arkodg/gateway that referenced this issue Jul 20, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit that referenced this issue Jul 21, 2023
* `egctl x translate` support for EnvoyPatchPolicy

Relates to #24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@arkodg arkodg modified the milestones: 0.5.0-rc1, 0.5.0 Jul 26, 2023
arkodg added a commit to arkodg/gateway that referenced this issue Jul 31, 2023
Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit that referenced this issue Aug 1, 2023
* Add user docs for EnvoyPatchPolicy

Relates to #24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* nits

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* wrap up

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* address comments && fix config

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@arkodg arkodg closed this as completed Aug 2, 2023
arkodg added a commit to arkodg/gateway that referenced this issue Aug 2, 2023
* Add user docs for EnvoyPatchPolicy

Relates to envoyproxy#24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* nits

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* wrap up

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* address comments && fix config

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
(cherry picked from commit 27b0939)
arkodg added a commit that referenced this issue Aug 2, 2023
* refactor: set defaults in Deployment, else k8s sets them for you, creating infinite reconciliation loop (#1594)

* fix: envoy proxy resource apply bug.

Signed-off-by: qicz <qiczzhu@gmail.com>

* update pointer.

Signed-off-by: qicz <qiczzhu@gmail.com>

* add comment

Signed-off-by: qicz <qiczzhu@gmail.com>

* update cm cmp logic.

Signed-off-by: qicz <qiczzhu@gmail.com>

* fix lint

Signed-off-by: qicz <qiczzhu@gmail.com>

* add probe field default value.

Signed-off-by: qicz <qiczzhu@gmail.com>

* fix uts

Signed-off-by: qicz <qiczzhu@gmail.com>

* align probe

Signed-off-by: qicz <qiczzhu@gmail.com>

* optimize deploy compare logic

Signed-off-by: qicz <qiczzhu@gmail.com>

* add compare deploy uts

Signed-off-by: qicz <qiczzhu@gmail.com>

* rm cm binarydata cmp

Signed-off-by: qicz <qiczzhu@gmail.com>

* rm deploy cmp logic

Signed-off-by: qicz <qiczzhu@gmail.com>

* fix ut

Signed-off-by: qicz <qiczzhu@gmail.com>

* fix lint

Signed-off-by: qicz <qiczzhu@gmail.com>

---------

Signed-off-by: qicz <qiczzhu@gmail.com>
Signed-off-by: qi <qiczzhu@gmail.com>
(cherry picked from commit 9ba9103)

* DeepCopy resources that require status updates (#1723)

* Was seeing constant churn between provider runner publishing resources
and gateway-api runner receiving them.

* Tried to debug it by printing the o/p of `cmp.Diff` between current
  and previous values
```
diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go
index 050394ba..50d09f6f 100644
--- a/internal/gatewayapi/runner/runner.go
+++ b/internal/gatewayapi/runner/runner.go
@@ -8,6 +8,7 @@ package runner
 import (
        "context"

+       "github.com/google/go-cmp/cmp"
        "k8s.io/apimachinery/pkg/runtime/schema"
        "sigs.k8s.io/gateway-api/apis/v1beta1"
        "sigs.k8s.io/yaml"
@@ -49,6 +50,7 @@ func (r *Runner) Start(ctx context.Context) error {
 }

 func (r *Runner) subscribeAndTranslate(ctx context.Context) {
+       prev := &gatewayapi.Resources{}
        message.HandleSubscription(r.ProviderResources.GatewayAPIResources.Subscribe(ctx),
                func(update message.Update[string, *gatewayapi.Resources]) {
                        val := update.Value
@@ -56,6 +58,9 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) {
                        if update.Delete || val == nil {
                                return
                        }
+                       diff := cmp.Diff(prev, val)
+                       r.Logger.WithValues("output", "diff").Info(diff)
+                       prev = val.DeepCopy()

                        // Translate and publish IRs.
                        t := &gatewayapi.Translator{
```

Here's the o/p and its empty
```
2023-07-27T23:55:29.795Z	INFO	gateway-api	runner/runner.go:62		{"runner": "gateway-api", "output": "diff"}
```

* Using a DeepCopy for resources that were updating the `Status`
  subresource seems to have solved the issue, which implies that
  watchable doesnt like clients to mutate the value, even though they
  are meant to be a `DeepCopy`

Fixes: #1715

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
(cherry picked from commit 5b72451)

* observability: add container port for metrics (#1736)

container port

Signed-off-by: zirain <zirain2009@gmail.com>
(cherry picked from commit 4bba03a)

* docs: Add user docs for EnvoyPatchPolicy (#1733)

* Add user docs for EnvoyPatchPolicy

Relates to #24

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* nits

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* wrap up

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* address comments && fix config

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
(cherry picked from commit 27b0939)

* e2e & misc fixes for EnvoyPatchPolicy (#1738)

* Add E2E for EnvoyPatchPolicy

* Use LocalReplyConfig to return a custom
status code `406` when there is no valid route match

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
(cherry picked from commit a7784c5)

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Co-authored-by: qi <qiczzhu@gmail.com>
Co-authored-by: zirain <zirain2009@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API-related issues area/extensions area/translator Issues related to Gateway's translation service, e.g. translating Gateway APIs into the IR. area/xds-server Issues related to the xDS Server used for managing Envoy configuration. documentation Improvements or additions to documentation kind/decision A record of a decision made by the community. kind/enhancement New feature or request no stalebot
Projects
None yet
Development

No branches or pull requests

8 participants