feat: New istio config format (knative-extensions#1247)

go.mod

@@ -7,6 +7,7 @@ require (
 	go.uber.org/zap v1.26.0
 	golang.org/x/sync v0.6.0
 	google.golang.org/protobuf v1.32.0
+	gopkg.in/yaml.v2 v2.4.0
 	istio.io/api v1.20.2
 	istio.io/client-go v1.20.2
 	k8s.io/api v0.28.5
@@ -82,7 +83,6 @@ require (
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20231212172506-995d672761c0 // indirect
 	google.golang.org/grpc v1.60.1 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
-	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/apiextensions-apiserver v0.28.5 // indirect
 	k8s.io/code-generator v0.28.5 // indirect

pkg/reconciler/ingress/config/istio.go

@@ -21,6 +21,7 @@ import (
 	"sort"
 	"strings"
 
+	"gopkg.in/yaml.v2"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/network"
@@ -38,6 +39,12 @@ const (
 	// localGatewayKeyPrefix is the prefix of all keys to configure Istio gateways for public & private Ingresses.
 	localGatewayKeyPrefix = "local-gateway."
 
+	// gatewayKey is the configmap key to configure Istio gateways for public Ingresses.
+	gatewayKey = "external-gateways"
+
+	// localGatewayKey is the configmap key to configure Istio gateways for public & private Ingresses.
+	localGatewayKey = "local-gateways"
+
 	// KnativeIngressGateway is the name of the ingress gateway
 	KnativeIngressGateway = "knative-ingress-gateway"
 
@@ -71,14 +78,34 @@ func defaultLocalGateways() []Gateway {
 type Gateway struct {
 	Namespace  string
 	Name       string
-	ServiceURL string
+	ServiceURL string `yaml:"service"`
 }
 
 // QualifiedName returns gateway name in '{namespace}/{name}' format.
 func (g Gateway) QualifiedName() string {
 	return g.Namespace + "/" + g.Name
 }
 
+func (g Gateway) Validate() error {
+	if g.Namespace == "" {
+		return fmt.Errorf("missing namespace")
+	}
+
+	if g.Name == "" {
+		return fmt.Errorf("missing name")
+	}
+
+	if g.ServiceURL == "" {
+		return fmt.Errorf("missing service")
+	}
+
+	if errs := validation.IsDNS1123Subdomain(strings.TrimSuffix(g.ServiceURL, ".")); len(errs) > 0 {
+		return fmt.Errorf("invalid gateway service format: %v", errs)
+	}
+
+	return nil
+}
+
 // Istio contains istio related configuration defined in the
 // istio config map.
 type Istio struct {
@@ -89,17 +116,120 @@ type Istio struct {
 	LocalGateways []Gateway
 }
 
-func parseGateways(configMap *corev1.ConfigMap, prefix string) ([]Gateway, error) {
+func (i Istio) Validate() error {
+	for _, gtw := range i.IngressGateways {
+		if err := gtw.Validate(); err != nil {
+			return fmt.Errorf("invalid gateway: %w", err)
+		}
+	}
+
+	for _, gtw := range i.LocalGateways {
+		if err := gtw.Validate(); err != nil {
+			return fmt.Errorf("invalid local gateway: %w", err)
+		}
+	}
+
+	return nil
+}
+
+// NewIstioFromConfigMap creates an Istio config from the supplied ConfigMap
+func NewIstioFromConfigMap(configMap *corev1.ConfigMap) (*Istio, error) {
+	ret, err := parseNewFormat(configMap)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse configmap: %w", err)
+	}
+
+	// If the new format is not defined, try to parse the old one
+	if len(ret.IngressGateways) == 0 && len(ret.LocalGateways) == 0 {
+		ret = parseOldFormat(configMap)
+	}
+
+	err = ret.Validate()
+	if err != nil {
+		return nil, fmt.Errorf("invalid configmap: %w", err)
+	}
+
+	return ret, nil
+}
+
+func parseNewFormat(configMap *corev1.ConfigMap) (*Istio, error) {
+	ret := &Istio{}
+
+	gatewaysStr, hasGateway := configMap.Data[gatewayKey]
+	localGatewaysStr, hasLocalGateway := configMap.Data[localGatewayKey]
+
+	// New format is partially defined
+	if (!hasGateway && hasLocalGateway) || (hasGateway && !hasLocalGateway) {
+		return nil, fmt.Errorf("config is not fully defined")
+	}
+
+	// New format is not defined
+	if !hasGateway && !hasLocalGateway {
+		return ret, nil
+	}
+
+	gateways, err := parseNewFormatGateways(gatewaysStr)
+	if err != nil {
+		return ret, fmt.Errorf("failed to parse gateways")
+	}
+
+	ret.IngressGateways = gateways
+
+	localGateways, err := parseNewFormatGateways(localGatewaysStr)
+	if err != nil {
+		return ret, fmt.Errorf("failed to parse local gateways")
+	}
+
+	if len(localGateways) > 1 {
+		return ret, fmt.Errorf("only one local gateways can be defined")
+	}
+
+	ret.LocalGateways = localGateways
+
+	return ret, nil
+}
+
+func parseNewFormatGateways(data string) ([]Gateway, error) {
+	ret := make([]Gateway, 0)
+
+	if len(data) == 0 {
+		return ret, nil
+	}
+
+	err := yaml.Unmarshal([]byte(data), &ret)
+	if err != nil {
+		return ret, fmt.Errorf("failed to unmarshal: %w", err)
+	}
+
+	return ret, nil
+}
+
+func parseOldFormat(configMap *corev1.ConfigMap) *Istio {
+	gateways := parseOldFormatGateways(configMap, gatewayKeyPrefix)
+	if len(gateways) == 0 {
+		gateways = defaultIngressGateways()
+	}
+
+	localGateways := parseOldFormatGateways(configMap, localGatewayKeyPrefix)
+	if len(localGateways) == 0 {
+		localGateways = defaultLocalGateways()
+	}
+
+	return &Istio{
+		IngressGateways: gateways,
+		LocalGateways:   localGateways,
+	}
+}
+
+func parseOldFormatGateways(configMap *corev1.ConfigMap, prefix string) []Gateway {
 	urls := map[string]string{}
 	gatewayNames := []string{}
 	for k, v := range configMap.Data {
 		if !strings.HasPrefix(k, prefix) || k == prefix {
 			continue
 		}
 		gatewayName, serviceURL := k[len(prefix):], v
-		if errs := validation.IsDNS1123Subdomain(strings.TrimSuffix(serviceURL, ".")); len(errs) > 0 {
-			return nil, fmt.Errorf("invalid gateway format: %v", errs)
-		}
+
 		gatewayNames = append(gatewayNames, gatewayName)
 		urls[gatewayName] = serviceURL
 	}
@@ -121,28 +251,5 @@ func parseGateways(configMap *corev1.ConfigMap, prefix string) ([]Gateway, error
 			ServiceURL: urls[gatewayName],
 		}
 	}
-	return gateways, nil
-}
-
-// NewIstioFromConfigMap creates an Istio config from the supplied ConfigMap
-func NewIstioFromConfigMap(configMap *corev1.ConfigMap) (*Istio, error) {
-	gateways, err := parseGateways(configMap, gatewayKeyPrefix)
-	if err != nil {
-		return nil, err
-	}
-	if len(gateways) == 0 {
-		gateways = defaultIngressGateways()
-	}
-	localGateways, err := parseGateways(configMap, localGatewayKeyPrefix)
-	if err != nil {
-		return nil, err
-	}
-	if len(localGateways) == 0 {
-		localGateways = defaultLocalGateways()
-	}
-
-	return &Istio{
-		IngressGateways: gateways,
-		LocalGateways:   localGateways,
-	}, nil
+	return gateways
 }