Skip to content

Commit

Permalink
Gateway filtering based on exposition annotation
Browse files Browse the repository at this point in the history 
* feat: Filter based on exposition annotation

* test: Add unit tests for gateway filtering based on annotations

* test: Add some unit test case if no gateway matched exposition value
  • Loading branch information
@pastequo
pastequo authored Oct 12, 2023
1 parent 00cc714 commit f2fc0b9
Show file tree
Hide file tree
Showing 10 changed files with 590 additions and 345 deletions.
49 changes: 46 additions & 3 deletions pkg/reconciler/ingress/config/istio.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"knative.dev/pkg/network"
"knative.dev/pkg/system"
Expand All @@ -38,6 +39,9 @@ const (
// localGatewayKeyPrefix is the prefix of all keys to configure Istio gateways for public & private Ingresses.
localGatewayKeyPrefix = "local-gateway."

// expositionKeyPrefix is the prefix of all keys to filter on Istio gateways.
expositionKeyPrefix = "exposition."

// KnativeIngressGateway is the name of the ingress gateway
KnativeIngressGateway = "knative-ingress-gateway"

Expand Down Expand Up @@ -69,9 +73,10 @@ func defaultLocalGateways() []Gateway {

// Gateway specifies the name of the Gateway and the K8s Service backing it.
type Gateway struct {
Namespace string
Name string
ServiceURL string
Namespace string
Name string
ServiceURL string
Expositions sets.Set[string]
}

// QualifiedName returns gateway name in '{namespace}/{name}' format.
Expand Down Expand Up @@ -103,6 +108,9 @@ func parseGateways(configMap *corev1.ConfigMap, prefix string) ([]Gateway, error
gatewayNames = append(gatewayNames, gatewayName)
urls[gatewayName] = serviceURL
}

expositions := parseExposition(configMap)

sort.Strings(gatewayNames)
gateways := make([]Gateway, len(gatewayNames))
for i, gatewayName := range gatewayNames {
Expand All @@ -120,10 +128,45 @@ func parseGateways(configMap *corev1.ConfigMap, prefix string) ([]Gateway, error
Name: name,
ServiceURL: urls[gatewayName],
}

if expositionKeys, ok := expositions[fmt.Sprintf("%s.%s", namespace, name)]; ok {
gateways[i].Expositions = expositionKeys
}
}
return gateways, nil
}

func parseExposition(configMap *corev1.ConfigMap) map[string]sets.Set[string] {
ret := make(map[string]sets.Set[string])

for k, v := range configMap.Data {
if !strings.HasPrefix(k, expositionKeyPrefix) || k == expositionKeyPrefix {
continue
}

gatewayName, expositionKeys := k[len(expositionKeyPrefix):], v

if !strings.Contains(gatewayName, ".") {
gatewayName = fmt.Sprintf("%s.%s", system.Namespace(), gatewayName)
}

expositions := strings.Split(expositionKeys, ",")
for _, expo := range expositions {
toAdd := strings.TrimSpace(expo)
if len(toAdd) == 0 {
continue
}
if _, ok := ret[gatewayName]; !ok {
ret[gatewayName] = sets.New[string]()
}

ret[gatewayName] = ret[gatewayName].Insert(toAdd)
}
}

return ret
}

// NewIstioFromConfigMap creates an Istio config from the supplied ConfigMap
func NewIstioFromConfigMap(configMap *corev1.ConfigMap) (*Istio, error) {
gateways, err := parseGateways(configMap, gatewayKeyPrefix)
Expand Down
159 changes: 159 additions & 0 deletions pkg/reconciler/ingress/config/istio_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"knative.dev/pkg/system"

. "knative.dev/pkg/configmap/testing"
Expand Down Expand Up @@ -222,6 +223,164 @@ func TestGatewayConfiguration(t *testing.T) {
"local-gateway.custom-namespace.invalid": "_invalid",
},
},
}, {
name: "local gateway configuration in custom namespace with valid url with exposition",
wantErr: false,
wantIstio: &Istio{
IngressGateways: defaultIngressGateways(),
LocalGateways: []Gateway{
{
Namespace: "custom-namespace-0",
Name: "custom-local-gateway-0",
ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local",
},
{
Namespace: "custom-namespace-1",
Name: "custom-local-gateway-1",
ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local",
Expositions: sets.New[string]("some-value-1"),
},
{
Namespace: "custom-namespace-2",
Name: "custom-local-gateway-2",
ServiceURL: "istio-ingressbackroad.istio-system.svc.cluster.local",
Expositions: sets.New[string]("some-value-2a", "some-value-2b"),
}},
},
config: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: system.Namespace(),
Name: IstioConfigName,
},
Data: map[string]string{
"local-gateway.custom-namespace-0.custom-local-gateway-0": "istio-ingressbackroad.istio-system.svc.cluster.local",
"local-gateway.custom-namespace-1.custom-local-gateway-1": "istio-ingressbackroad.istio-system.svc.cluster.local",
"local-gateway.custom-namespace-2.custom-local-gateway-2": "istio-ingressbackroad.istio-system.svc.cluster.local",
"exposition.custom-namespace-1.custom-local-gateway-1": "some-value-1",
"exposition.custom-namespace-2.custom-local-gateway-2": "some-value-2a,some-value-2b",
},
},
}, {
name: "gateway configuration defined with/without namespace with exposition defined with/without namespace",
wantErr: false,
wantIstio: &Istio{
IngressGateways: []Gateway{
{
Namespace: "knative-testing",
Name: "custom-gateway-wo-wi",
ServiceURL: "istio-ingressfreeway.istio-system.svc.cluster.local",
Expositions: sets.New[string]("some-value-wo-wi"),
},
{
Namespace: "knative-testing",
Name: "custom-gateway-wo-wo",
ServiceURL: "istio-ingressfreeway.istio-system.svc.cluster.local",
Expositions: sets.New[string]("some-value-wo-wo"),
},
{
Namespace: "knative-testing",
Name: "custom-gateway-wi-wi",
ServiceURL: "istio-ingressfreeway.istio-system.svc.cluster.local",
Expositions: sets.New[string]("some-value-wi-wi"),
},
{
Namespace: "knative-testing",
Name: "custom-gateway-wi-wo",
ServiceURL: "istio-ingressfreeway.istio-system.svc.cluster.local",
Expositions: sets.New[string]("some-value-wi-wo"),
}},
LocalGateways: defaultLocalGateways(),
},
config: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: system.Namespace(),
Name: IstioConfigName,
},
Data: map[string]string{
"gateway.custom-gateway-wo-wo": "istio-ingressfreeway.istio-system.svc.cluster.local",
"gateway.custom-gateway-wo-wi": "istio-ingressfreeway.istio-system.svc.cluster.local",
"gateway.knative-testing.custom-gateway-wi-wo": "istio-ingressfreeway.istio-system.svc.cluster.local",
"gateway.knative-testing.custom-gateway-wi-wi": "istio-ingressfreeway.istio-system.svc.cluster.local",
"exposition.custom-gateway-wo-wo": "some-value-wo-wo",
"exposition.knative-testing.custom-gateway-wo-wi": "some-value-wo-wi",
"exposition.custom-gateway-wi-wo": "some-value-wi-wo",
"exposition.knative-testing.custom-gateway-wi-wi": "some-value-wi-wi",
},
},
}, {
name: "Same exposition defined for local and non-local gateways",
wantErr: false,
wantIstio: &Istio{
IngressGateways: []Gateway{
{
Namespace: "knative-testing",
Name: "custom-gateway-no-exposition",
ServiceURL: "istio-ingress1.istio-system.svc.cluster.local",
},
{
Namespace: "knative-testing",
Name: "custom-gateway-with-exposition",
ServiceURL: "istio-ingress2.istio-system.svc.cluster.local",
Expositions: sets.New[string]("some-value"),
},
},
LocalGateways: []Gateway{
{
Namespace: "knative-testing",
Name: "custom-gateway-no-exposition",
ServiceURL: "istio-ingress1.istio-system.svc.cluster.local",
},
{
Namespace: "knative-testing",
Name: "custom-gateway-with-exposition",
ServiceURL: "istio-ingress2.istio-system.svc.cluster.local",
Expositions: sets.New[string]("some-value"),
},
},
},
config: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: system.Namespace(),
Name: IstioConfigName,
},
Data: map[string]string{
"gateway.custom-gateway-no-exposition": "istio-ingress1.istio-system.svc.cluster.local",
"gateway.custom-gateway-with-exposition": "istio-ingress2.istio-system.svc.cluster.local",
"local-gateway.custom-gateway-no-exposition": "istio-ingress1.istio-system.svc.cluster.local",
"local-gateway.custom-gateway-with-exposition": "istio-ingress2.istio-system.svc.cluster.local",
"exposition.custom-gateway-with-exposition": "some-value",
},
},
}, {
name: "Exposition defined but referencing no known gateways",
wantErr: false,
wantIstio: &Istio{
IngressGateways: []Gateway{
{
Namespace: "knative-testing",
Name: "custom-gateway",
ServiceURL: "istio-ingress.istio-system.svc.cluster.local",
},
},
LocalGateways: []Gateway{
{
Namespace: "knative-testing",
Name: "custom-local-gateway",
ServiceURL: "istio-ingress.istio-system.svc.cluster.local",
},
},
},
config: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: system.Namespace(),
Name: IstioConfigName,
},
Data: map[string]string{
"gateway.custom-gateway": "istio-ingress.istio-system.svc.cluster.local",
"local-gateway.custom-local-gateway": "istio-ingress.istio-system.svc.cluster.local",
"exposition.unknown-value": "some-value",
},
},
}}

for _, tt := range gatewayConfigTests {
Expand Down
72 changes: 5 additions & 67 deletions pkg/reconciler/ingress/ingress.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,7 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
ing.Status.InitializeConditions()
logger.Infof("Reconciling ingress: %#v", ing)

defaultGateways, err := computeDefaultGateways(ctx, ing)
if err != nil {
return fmt.Errorf("failed to compute default gateways: %w", err)
}
defaultGateways := resources.QualifiedGatewayNamesFromContext(ctx, ing)

gatewayNames := map[v1alpha1.IngressVisibility]sets.Set[string]{}
gatewayNames[v1alpha1.IngressVisibilityClusterLocal] = defaultGateways[v1alpha1.IngressVisibilityClusterLocal]
Expand All @@ -134,7 +131,7 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
if err != nil {
return err
}
targetWildcardSecrets, err := resources.MakeWildcardSecrets(ctx, wildcardSecrets)
targetWildcardSecrets, err := resources.MakeWildcardSecrets(ctx, wildcardSecrets, ing)
if err != nil {
return err
}
Expand Down Expand Up @@ -226,8 +223,8 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
}

if ready {
publicLbs := getLBStatus(publicGatewayServiceURLFromContext(ctx))
privateLbs := getLBStatus(privateGatewayServiceURLFromContext(ctx))
publicLbs := getLBStatus(resources.PublicGatewayServiceURLFromContext(ctx, ing))
privateLbs := getLBStatus(resources.PrivateGatewayServiceURLFromContext(ctx, ing))
ing.Status.MarkLoadBalancerReady(publicLbs, privateLbs)
} else {
ing.Status.MarkLoadBalancerNotReady()
Expand Down Expand Up @@ -373,7 +370,7 @@ func (r *Reconciler) reconcileDeletion(ctx context.Context, ing *v1alpha1.Ingres

errs := []error{}
for _, tls := range ing.Spec.TLS {
nameNamespaces, err := resources.GetIngressGatewaySvcNameNamespaces(ctx)
nameNamespaces, err := resources.GetIngressGatewaySvcNameNamespaces(ctx, ing)
if err != nil {
errs = append(errs, err)
continue
Expand Down Expand Up @@ -441,41 +438,6 @@ func (r *Reconciler) GetVirtualServiceLister() istiolisters.VirtualServiceLister
return r.virtualServiceLister
}

// qualifiedGatewayNamesFromContext get gateway names from context
func qualifiedGatewayNamesFromContext(ctx context.Context) map[v1alpha1.IngressVisibility]sets.Set[string] {
ci := config.FromContext(ctx).Istio
publicGateways := sets.New[string]()
for _, gw := range ci.IngressGateways {
publicGateways.Insert(gw.QualifiedName())
}

privateGateways := sets.New[string]()
for _, gw := range ci.LocalGateways {
privateGateways.Insert(gw.QualifiedName())
}

return map[v1alpha1.IngressVisibility]sets.Set[string]{
v1alpha1.IngressVisibilityExternalIP: publicGateways,
v1alpha1.IngressVisibilityClusterLocal: privateGateways,
}
}

func publicGatewayServiceURLFromContext(ctx context.Context) string {
cfg := config.FromContext(ctx).Istio
if len(cfg.IngressGateways) > 0 {
return cfg.IngressGateways[0].ServiceURL
}
return ""
}

func privateGatewayServiceURLFromContext(ctx context.Context) string {
cfg := config.FromContext(ctx).Istio
if len(cfg.LocalGateways) > 0 {
return cfg.LocalGateways[0].ServiceURL
}
return ""
}

// getLBStatus gets the LB Status.
func getLBStatus(gatewayServiceURL string) []v1alpha1.LoadBalancerIngressStatus {
// The Ingress isn't load-balanced by any particular
Expand Down Expand Up @@ -509,27 +471,3 @@ func isIngressPublic(ing *v1alpha1.Ingress) bool {
}
return false
}

func computeDefaultGateways(ctx context.Context, ing *v1alpha1.Ingress) (map[v1alpha1.IngressVisibility]sets.String, error) {
ret := qualifiedGatewayNamesFromContext(ctx) // gateways from config

for _, visibility := range []v1alpha1.IngressVisibility{v1alpha1.IngressVisibilityClusterLocal, v1alpha1.IngressVisibilityExternalIP} {
gateways, err := resources.GetGatewaysFromAnnotations(ing, visibility)
if err != nil {
return nil, fmt.Errorf("failed to get %s gateways from annotation: %w", visibility, err)
}

// Ensure all gateways are known in the configuration
unknownGateways := gateways.Difference(ret[visibility])
if unknownGateways.Len() > 0 {
return nil, fmt.Errorf("following qualified name(s) aren't defined in the configuration (%s): %v", visibility, unknownGateways.List())
}

// If ingress specifies gateways, restrict to them
if gateways.Len() > 0 {
ret[visibility] = gateways
}
}

return ret, nil
}
Loading

0 comments on commit f2fc0b9

Please sign in to comment.