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

Add support for using gateway.spec.addresses as service external ips #1322

Merged
merged 13 commits into from
Apr 27, 2023
Merged
42 changes: 42 additions & 0 deletions internal/gatewayapi/address.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright Envoy Gateway Authors
// SPDX-License-Identifier: Apache-2.0
// The full text of the Apache license is available in the LICENSE file at
// the root of the repo.

package gatewayapi

import (
"net/netip"

"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/gateway-api/apis/v1beta1"
)

var _ AddressesTranslator = (*Translator)(nil)

type AddressesTranslator interface {
ProcessAddresses(gateways []*GatewayContext, xdsIR XdsIRMap, infraIR InfraIRMap, resources *Resources)
}

func (t *Translator) ProcessAddresses(gateways []*GatewayContext, xdsIR XdsIRMap, infraIR InfraIRMap, resources *Resources) {
for _, gateway := range gateways {
// Infra IR already exist
irKey := irStringKey(gateway.Gateway)
gwInfraIR := infraIR[irKey]

ipAddr := sets.Set[string]{}

for _, addr := range gateway.Spec.Addresses {
switch *addr.Type {
case v1beta1.IPAddressType:
if _, err := netip.ParseAddr(addr.Value); err == nil {
ipAddr.Insert(addr.Value)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we expose the error in the status condition ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, we cannot expose this error in the status condition.

becasue when we apply an invalid IP address (like a.b.c.d) to Gateway, it will fail to create a Gateway Service,

func (i *Infra) createOrUpdateService(ctx context.Context, svc *corev1.Service) error {
current := &corev1.Service{}
key := types.NamespacedName{
Namespace: svc.Namespace,
Name: svc.Name,
}
if err := i.Client.Get(ctx, key, current); err != nil {
// Create if not found.
if kerrors.IsNotFound(err) {
if err := i.Client.Create(ctx, svc); err != nil {
return fmt.Errorf("failed to create service %s/%s: %w",
svc.Namespace, svc.Name, err)

thus we cannot get this Service when setting the status condition:

func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1b1.Gateway, svc *corev1.Service, deployment *appsv1.Deployment) {
var addresses, hostnames []string
// Update the status addresses field.
if svc != nil {

IMO, we can solve this by validating the Gateway.Spec.Addresses before creating the Service, since the GW API does not provide any validation webhook on this field, should we validate it ourselves or raise an issue in upstream?

cc @arkodg

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for raising an issue upstream !

}
}
}

if ip := sets.List(ipAddr); len(ip) > 0 {
shawnh2 marked this conversation as resolved.
Show resolved Hide resolved
gwInfraIR.Proxy.Addresses = ip
}
}
}
4 changes: 4 additions & 0 deletions internal/gatewayapi/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type TranslatorManager interface {

RoutesTranslator
ListenersTranslator
AddressesTranslator
FiltersTranslator
}

Expand Down Expand Up @@ -119,6 +120,9 @@ func (t *Translator) Translate(resources *Resources) *TranslateResult {
// Process all Listeners for all relevant Gateways.
t.ProcessListeners(gateways, xdsIR, infraIR, resources)

// Process all Addresses for all relevant Gateways.
t.ProcessAddresses(gateways, xdsIR, infraIR, resources)

// Process all relevant HTTPRoutes.
httpRoutes := t.ProcessHTTPRoutes(resources.HTTPRoutes, gateways, resources, xdsIR)

Expand Down
5 changes: 5 additions & 0 deletions internal/infrastructure/kubernetes/proxy_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,14 @@ func (i *Infra) expectedProxyService(infra *ir.Infra) (*corev1.Service, error) {
if envoyServiceConfig.Annotations != nil {
annotations = envoyServiceConfig.Annotations
}

// Set the spec of envoy gateway service
serviceSpec := expectedServiceSpec(envoyServiceConfig.Type)
serviceSpec.Ports = ports
serviceSpec.Selector = getSelector(labels).MatchLabels
if len(infra.Proxy.Addresses) > 0 {
serviceSpec.ExternalIPs = infra.Proxy.Addresses
}

svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Expand Down
3 changes: 3 additions & 0 deletions internal/ir/infra.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type ProxyInfra struct {
Config *v1alpha1.EnvoyProxy
// Listeners define the listeners exposed by the proxy infrastructure.
Listeners []ProxyListener
// Addresses contain the external addresses this gateway has been
// requested to be available at.
Addresses []string
}

// InfraMetadata defines metadata for the managed proxy infrastructure.
Expand Down
4 changes: 4 additions & 0 deletions internal/status/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1b1.Gateway, svc *corev1.S
}
}

for _, eip := range svc.Spec.ExternalIPs {
addresses = append(addresses, eip)
}

var gwAddresses []gwapiv1b1.GatewayAddress
for i := range addresses {
addr := gwapiv1b1.GatewayAddress{
Expand Down