diff --git a/pkg/reconciler/ingress/config/istio.go b/pkg/reconciler/ingress/config/istio.go index 70702801d8..d43d9db883 100644 --- a/pkg/reconciler/ingress/config/istio.go +++ b/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,7 +78,7 @@ func defaultLocalGateways() []Gateway { type Gateway struct { Namespace string Name string - ServiceURL string + ServiceURL string `yaml:"service"` } // QualifiedName returns gateway name in '{namespace}/{name}' format. @@ -79,6 +86,26 @@ 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,7 +116,112 @@ 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 { @@ -97,9 +229,7 @@ func parseGateways(configMap *corev1.ConfigMap, prefix string) ([]Gateway, error 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 } diff --git a/pkg/reconciler/ingress/config/istio_test.go b/pkg/reconciler/ingress/config/istio_test.go index a6a464613b..0dd34eb29f 100644 --- a/pkg/reconciler/ingress/config/istio_test.go +++ b/pkg/reconciler/ingress/config/istio_test.go @@ -17,6 +17,7 @@ limitations under the License. package config import ( + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -56,7 +57,7 @@ func TestGatewayConfiguration(t *testing.T) { gatewayConfigTests := []struct { name string wantErr bool - wantIstio interface{} + wantIstio *Istio config *corev1.ConfigMap }{{ name: "gateway configuration with no network input", @@ -71,9 +72,8 @@ func TestGatewayConfiguration(t *testing.T) { }, }, }, { - name: "gateway configuration with invalid url", - wantErr: true, - wantIstio: (*Istio)(nil), + name: "gateway configuration with invalid url", + wantErr: true, config: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.Namespace(), @@ -84,8 +84,7 @@ func TestGatewayConfiguration(t *testing.T) { }, }, }, { - name: "gateway configuration with valid url", - wantErr: false, + name: "gateway configuration with valid url", wantIstio: &Istio{ IngressGateways: []Gateway{{ Namespace: "knative-testing", @@ -104,8 +103,7 @@ func TestGatewayConfiguration(t *testing.T) { }, }, }, { - name: "gateway configuration with valid url having a dot at the end", - wantErr: false, + name: "gateway configuration with valid url having a dot at the end", wantIstio: &Istio{ IngressGateways: []Gateway{{ Namespace: "knative-testing", @@ -124,8 +122,7 @@ func TestGatewayConfiguration(t *testing.T) { }, }, }, { - name: "gateway configuration in custom namespace with valid url", - wantErr: false, + name: "gateway configuration in custom namespace with valid url", wantIstio: &Istio{ IngressGateways: []Gateway{{ Namespace: "custom-namespace", @@ -144,9 +141,8 @@ func TestGatewayConfiguration(t *testing.T) { }, }, }, { - name: "gateway configuration in custom namespace with invalid url", - wantErr: true, - wantIstio: (*Istio)(nil), + name: "gateway configuration in custom namespace with invalid url", + wantErr: true, config: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.Namespace(), @@ -157,8 +153,7 @@ func TestGatewayConfiguration(t *testing.T) { }, }, }, { - name: "local gateway configuration with valid url", - wantErr: false, + name: "local gateway configuration with valid url", wantIstio: &Istio{ IngressGateways: defaultIngressGateways(), LocalGateways: []Gateway{{ @@ -177,9 +172,8 @@ func TestGatewayConfiguration(t *testing.T) { }, }, }, { - name: "local gateway configuration with invalid url", - wantErr: true, - wantIstio: (*Istio)(nil), + name: "local gateway configuration with invalid url", + wantErr: true, config: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.Namespace(), @@ -190,8 +184,7 @@ func TestGatewayConfiguration(t *testing.T) { }, }, }, { - name: "local gateway configuration in custom namespace with valid url", - wantErr: false, + name: "local gateway configuration in custom namespace with valid url", wantIstio: &Istio{ IngressGateways: defaultIngressGateways(), LocalGateways: []Gateway{{ @@ -210,9 +203,8 @@ func TestGatewayConfiguration(t *testing.T) { }, }, }, { - name: "local gateway configuration in custom namespace with invalid url", - wantErr: true, - wantIstio: (*Istio)(nil), + name: "local gateway configuration in custom namespace with invalid url", + wantErr: true, config: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.Namespace(), @@ -222,6 +214,174 @@ func TestGatewayConfiguration(t *testing.T) { "local-gateway.custom-namespace.invalid": "_invalid", }, }, + }, { + name: "new format alone", + wantIstio: &Istio{ + IngressGateways: []Gateway{{ + Namespace: "namespace1", + Name: "gateway1", + ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local", + }}, + LocalGateways: []Gateway{{ + Namespace: "namespace2", + Name: "gateway2", + ServiceURL: "istio-local-gateway.istio-system.svc.cluster.local", + }}, + }, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "external-gateways": generateNewConfig(Gateway{Namespace: "namespace1", Name: "gateway1", ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local"}), + "local-gateways": generateNewConfig(Gateway{Namespace: "namespace2", Name: "gateway2", ServiceURL: "istio-local-gateway.istio-system.svc.cluster.local"}), + }, + }, + }, { + name: "new & old format", + wantIstio: &Istio{ + IngressGateways: []Gateway{{ + Namespace: "namespace1", + Name: "gateway1", + ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local", + }}, + LocalGateways: []Gateway{{ + Namespace: "namespace2", + Name: "gateway2", + ServiceURL: "istio-local-gateway.istio-system.svc.cluster.local", + }}, + }, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "external-gateways": generateNewConfig(Gateway{Namespace: "namespace1", Name: "gateway1", ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local"}), + "local-gateways": generateNewConfig(Gateway{Namespace: "namespace2", Name: "gateway2", ServiceURL: "istio-local-gateway.istio-system.svc.cluster.local"}), + "local-gateway.custom-namespace.custom-local-gateway": "istio-ingressbackroad.istio-system.svc.cluster.local", + "gateway.custom-namespace.custom-gateway": "istio-ingressfreeway.istio-system.svc.cluster.local", + }, + }, + }, { + name: "new format - double local gateway", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "local-gateways": generateNewConfig( + Gateway{Namespace: "namespace1", Name: "gateway1", ServiceURL: "istio-local-gateway1.istio-system.svc.cluster.local"}, + Gateway{Namespace: "namespace2", Name: "gateway2", ServiceURL: "istio-local-gateway2.istio-system.svc.cluster.local"}, + ), + "external-gateways": generateNewConfig(Gateway{Namespace: "namespace1", Name: "gateway1", ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local"}), + }, + }, + }, { + name: "new format - invalid URL", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "external-gateways": generateNewConfig(Gateway{Namespace: "namespace1", Name: "gateway1", ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local"}), + "local-gateways": generateNewConfig(Gateway{Namespace: "namespace2", Name: "gateway2", ServiceURL: "_invalid"}), + }, + }, + }, { + name: "new format - missing service", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "external-gateways": generateNewConfig(Gateway{Namespace: "namespace1", Name: "gateway1", ServiceURL: ""}), + "local-gateways": generateNewConfig(Gateway{Namespace: "namespace2", Name: "gateway2", ServiceURL: "istio-local-gateway.istio-system.svc.cluster.local"}), + }, + }, + }, { + name: "new format - missing name", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "external-gateways": generateNewConfig(Gateway{Namespace: "namespace1", Name: "", ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local"}), + "local-gateways": generateNewConfig(Gateway{Namespace: "namespace2", Name: "gateway2", ServiceURL: "istio-local-gateway.istio-system.svc.cluster.local"}), + }, + }, + }, { + name: "new format - missing namespace", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "external-gateways": generateNewConfig(Gateway{Namespace: "namespace1", Name: "gateway1", ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local"}), + "local-gateways": generateNewConfig(Gateway{Namespace: "", Name: "gateway2", ServiceURL: "istio-local-gateway.istio-system.svc.cluster.local"}), + }, + }, + }, { + name: "new format - invalid local yaml", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "external-gateways": generateNewConfig(Gateway{Namespace: "namespace1", Name: "gateway1", ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local"}), + "local-gateways": "notYAML", + }, + }, + }, { + name: "new format - invalid external yaml", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "external-gateways": "notYAML", + "local-gateways": generateNewConfig(Gateway{Namespace: "namespace2", Name: "gateway2", ServiceURL: "istio-local-gateway.istio-system.svc.cluster.local"}), + }, + }, + }, { + name: "new format - missing external gateway configuration", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "local-gateways": generateNewConfig(Gateway{Namespace: "namespace2", Name: "gateway2", ServiceURL: "istio-local-gateway.istio-system.svc.cluster.local"}), + }, + }, + }, { + name: "new format - missing local gateway configuration", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "external-gateways": generateNewConfig(Gateway{Namespace: "namespace2", Name: "gateway2", ServiceURL: "istio-gateway.istio-system.svc.cluster.local"}), + }, + }, }} for _, tt := range gatewayConfigTests { @@ -237,3 +397,17 @@ func TestGatewayConfiguration(t *testing.T) { }) } } + +func generateNewConfig(gtws ...Gateway) string { + ret := "" + + for _, gtw := range gtws { + if ret != "" { + ret = fmt.Sprintf("%s\n", ret) + } + + ret = fmt.Sprintf("%s- namespace: %s\n name: %s\n service: %s", ret, gtw.Namespace, gtw.Name, gtw.ServiceURL) + } + + return ret +}