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

Override default Envoy Proxy Bootstrap Configuration #31

Closed
7 tasks done
arkodg opened this issue Apr 28, 2022 · 13 comments · Fixed by #1333
Closed
7 tasks done

Override default Envoy Proxy Bootstrap Configuration #31

arkodg opened this issue Apr 28, 2022 · 13 comments · Fixed by #1333
Assignees
Labels
area/api API-related issues area/egctl area/extensions area/ir Issues related to Gateway's internal representation, e.g. data model. 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. kind/enhancement New feature or request priority/high Label used to express the "high" priority level
Milestone

Comments

@arkodg
Copy link
Contributor

arkodg commented Apr 28, 2022

#24 highlights the need for users to override xDS resources configured by
Envoy Gateway. Raising this issue to track how do you express this intent for the Envoy Proxy Bootstrap Config using the Gateway API

here's how popular OSS projects do it

Updated

@arkodg
Copy link
Contributor Author

arkodg commented Apr 28, 2022

Some options, we could provide some or all of these knobs

  • Support custom Envoy Proxy Bootstrap patches/configs using a field within the Envoy Gateway Bootstrap Config
  • Support Support custom Envoy Proxy Bootstrap patches/configs using a field within the Gateway Class Parameter Reference Object

@arkodg arkodg changed the title [xDS] Override default Envoy Proxy Bootstrap Configuration using Gateway API [xDS] Override default Envoy Proxy Bootstrap Configuration Apr 28, 2022
@danehans
Copy link
Contributor

From #24 (comment), I like all EG or all BYO bootstrap.

@danehans danehans added the kind/question Further information is requested label Apr 29, 2022
@danehans danehans modified the milestone: v0 Release Apr 29, 2022
@danehans danehans changed the title [xDS] Override default Envoy Proxy Bootstrap Configuration Override default Envoy Proxy Bootstrap Configuration May 11, 2022
@danehans danehans added the area/xds-server Issues related to the xDS Server used for managing Envoy configuration. label May 11, 2022
@github-actions github-actions bot added the stale label Jul 1, 2022
@github-actions github-actions bot closed this as completed Jul 8, 2022
@arkodg arkodg removed the stale label Sep 7, 2022
@arkodg arkodg reopened this Oct 1, 2022
@arkodg arkodg added this to the 0.3.0-rc.1 milestone Oct 1, 2022
@arkodg arkodg added the help wanted Extra attention is needed label Oct 20, 2022
@Xunzhuo Xunzhuo self-assigned this Oct 20, 2022
@danehans
Copy link
Contributor

Raising this issue to track how do you express this intent for the Envoy Proxy Bootstrap Config using the Gateway API

Currently, the bootstrap config is managed as a go template rendered by the Infra Manager and referenced as --config-yaml by the Envoy pod. As the Envoy Gateway Configuration API Design states:

... the managed Envoy proxies are configured dynamically through Kubernetes resources, primarily Gateway API objects. Optionally, the Envoy proxies can be configured by referencing the EnvoyProxy custom resource (CR) through spec.parametersRef of the managed GatewayClass.

I suggest extending the EnvoyProxy API type to support this use case. For example:

// EnvoyProxySpec defines the desired state of EnvoyProxy.
type EnvoyProxySpec struct {
	// Bootstrap defines the desired state of the Envoy bootstrap config file.
        // If unspecified, default bootstrap config parameters are used.
	//
	// +optional
	Bootstrap *Bootstrap `json:"bootstrap,omitempty"`
}

// Bootstrap defines the desired state of the Envoy bootstrap configuration file.
type Bootstrap struct {
	// Xds defines xDS configuration parameters...
        // If unspecified, default bootstrap config parameters are used.
	//
	// +optional
	Xds *Xds `json:"xds,omitempty"`
}

// Xds defines desired xDS configuration parameters.
type Xds struct {
	// Address defines the address of the xDS server...
        // If unspecified, "envoy-gateway" is used.
	//
	// +optional
	Address *XdsAddress `json:"address,omitempty"`

	// Port defines the port of the xDS server...
        // If unspecified, 18000 is used.
	//
	// +optional
	Port *XdsPortNumber `json:"port,omitempty"`
}

type XdsAddress string
type XdsPortNumber int32

A user would then create the EnvoyProxy CRD and a GatewayClass that references it. For example:

apiVersion: config.gateway.envoyproxy.io/v1alpha1
kind: EnvoyProxy
metadata:
  name: example
  namespace: envoy-gateway-system
spec:
  bootstrap:
    xds:
      address: host.docker.internal # used to run EG locally
      port: 1234
  ...
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: GatewayClass
metadata:
  name: eg
spec:
  controllerName: gateway.envoyproxy.io/gatewayclass-controller
  parametersRef:
    group: config.gateway.envoyproxy.io
    kind: EnvoyProxy
    name: example
    namespace: envoy-gateway-system

Envoy Gateway will need to add support for gatewayclass.spec.parametersRef, get the referrenced EnvoyProxy, and validate its fields. For example, the EnvoyProxy should be rejected and surface appropriate status conditions if bootstrap.xds.address and/or bootstrap.xds.port do not match Envoy Gateway's config.

A drawback of this approach is that EG manages only one GatewayClass and all Gateways will use the same bootstrap config.

@arkodg
Copy link
Contributor Author

arkodg commented Oct 24, 2022

@danehans I agree with where the Bootstrap content must be specified - within EnvoyProxySpec
But I do think we should start by allowing the user to completely specify a new bootstrap spec, rather than just edit the xds server info
To make this easier, envoy-gateway default xds-bootstrap (something similar) could help the user understand what is used by default

@danehans
Copy link
Contributor

@arkodg thanks for providing feedback.

The following are a few concerns that come to mind for "... allowing the user to completely specify a new bootstrap spec":

  1. If we will eventually support managing bootstrap config through EnvoyProxySpec, EG will have two ways to accomplish the same goal.
  2. Since EG CLI flags should also be supported by the EG config file, we would pollute the EnvoyGateway API with Envoy Proxy-specific config. The EnvoyGateway API should be for configuring Envoy Gateway and not the managed Envoy proxies.
  3. Supporting a full-blown replacement of the bootstrap config seems error-prone and a support burden on maintainers.

@arkodg
Copy link
Contributor Author

arkodg commented Oct 25, 2022

  1. Supporting a full-blown replacement of the bootstrap config seems error-prone and a support burden on maintainers.

I think we should better understand what the (advanced) users would like and what the project can provide.
#31 (comment) suggests we were planning on starting with all BYO bootstrap

@danehans
Copy link
Contributor

I think we should better understand what the (advanced) users would like and what the project can provide.
#31 (comment) suggests we were planning on starting with all BYO bootstrap

I consider ^ a discussion more than a plan. v0.3.0 planning will take place post-KubeCon NA 2022. Let's discuss this issue during planning to try and achieve a concensensouse on the path forward.

@arkodg
Copy link
Contributor Author

arkodg commented Oct 26, 2022

sg @danehans
also linking the extensibility goals section https://github.com/envoyproxy/gateway/blob/main/GOALS.md#extensibility here which should help drive the discussion, and would need to be revised if its no longer valid

@arkodg arkodg modified the milestones: 0.3.0-rc.1, Backlog Nov 3, 2022
@Xunzhuo Xunzhuo removed the help wanted Extra attention is needed label Nov 5, 2022
@github-actions
Copy link

github-actions bot commented Dec 5, 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 Dec 5, 2022
@github-actions
Copy link

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.

@arkodg arkodg added kind/enhancement New feature or request area/api API-related issues area/ir Issues related to Gateway's internal representation, e.g. data model. area/translator Issues related to Gateway's translation service, e.g. translating Gateway APIs into the IR. area/extensions priority/high Label used to express the "high" priority level area/egctl labels Mar 2, 2023
arkodg added a commit to arkodg/gateway that referenced this issue Mar 3, 2023
Relates to envoyproxy#31

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

Relates to #31

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

* more clarification in comments

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

* add optional tag

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

---------

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

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

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit that referenced this issue Mar 9, 2023
* validation for bootstrap within EnvoyProxy res

Relates to #31

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

* use embed

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit that referenced this issue Apr 6, 2023
Refactor EnvoyProxy's Validate API
If a GatewayClass references a EnvoyProxy resource, ensure that that the GatewayClass status condition is based off the EnvoyProxy validation result
Relates to #31 

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

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

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

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

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Alice-Lilith pushed a commit that referenced this issue Apr 13, 2023
* Add EnvoyProxy resource validation to `egctl translate`

Relates to #31

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

* fix logic

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

* add support for default EnvoyProxy

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

* lint

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

* fix yaml

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

* another test

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

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@arkodg arkodg modified the milestones: 0.4.0-rc.1, 0.4.0 Apr 13, 2023
@arkodg
Copy link
Contributor Author

arkodg commented Apr 13, 2023

keeping this open until we wrap up docs

arkodg added a commit to arkodg/gateway that referenced this issue Apr 19, 2023
Fixes: envoyproxy#31

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
zirain added a commit that referenced this issue Apr 20, 2023
* Update docs for xds bootstrap

Fixes: #31

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

* fix envoy proxy doc link

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

* lint

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

* lint

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

* undo

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

* Update docs/latest/user/egctl.md

Co-authored-by: zirain <zirain2009@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.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/egctl area/extensions area/ir Issues related to Gateway's internal representation, e.g. data model. 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. kind/enhancement New feature or request priority/high Label used to express the "high" priority level
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants