From fcabfa59b1d1a8ce8c3a66b6f577d93a85ecb7bb Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 28 Jun 2022 05:36:03 -0400 Subject: [PATCH 01/10] Add support for TLS (conformance test passes) --- config/200-clusterrole.yaml | 5 +- hack/test-env.sh | 4 +- pkg/reconciler/ingress/controller.go | 8 +- pkg/reconciler/ingress/controller_test.go | 1 + pkg/reconciler/ingress/ingress.go | 28 ++- pkg/reconciler/ingress/ingress_test.go | 173 ++++++++++++++++++ pkg/reconciler/ingress/reconcile_resources.go | 160 +++++++++++++++- pkg/reconciler/ingress/resources/httproute.go | 82 ++++----- .../ingress/resources/reference_grant.go | 60 ++++++ pkg/reconciler/testing/listers.go | 8 + 10 files changed, 479 insertions(+), 50 deletions(-) create mode 100644 pkg/reconciler/ingress/resources/reference_grant.go diff --git a/config/200-clusterrole.yaml b/config/200-clusterrole.yaml index f8b671d9d..f99eb9763 100644 --- a/config/200-clusterrole.yaml +++ b/config/200-clusterrole.yaml @@ -39,5 +39,8 @@ metadata: app.kubernetes.io/version: devel rules: - apiGroups: ["gateway.networking.k8s.io"] - resources: ["httproutes", "gateways"] + resources: ["httproutes", "referencepolicies"] verbs: ["get", "list", "create", "update", "delete", "patch", "watch"] + - apiGroups: ["gateway.networking.k8s.io"] + resources: ["gateways"] + verbs: ["get", "list", "update", "patch", "watch"] diff --git a/hack/test-env.sh b/hack/test-env.sh index 7b6ae94fd..c1714548f 100755 --- a/hack/test-env.sh +++ b/hack/test-env.sh @@ -16,6 +16,6 @@ export GATEWAY_API_VERSION="v0.4.3" export ISTIO_VERSION="1.13.2" -export ISTIO_UNSUPPORTED_TESTS="tls,retry,httpoption,host-rewrite" +export ISTIO_UNSUPPORTED_TESTS="retry,httpoption,host-rewrite" export CONTOUR_VERSION="v1.21.0" -export CONTOUR_UNSUPPORTED_TESTS="tls,retry,httpoption,basics/http2,websocket,websocket/split,grpc,grpc/split,visibility/path,visibility,update,host-rewrite" +export CONTOUR_UNSUPPORTED_TESTS="retry,httpoption,basics/http2,websocket,websocket/split,grpc,grpc/split,visibility/path,visibility,update,host-rewrite" diff --git a/pkg/reconciler/ingress/controller.go b/pkg/reconciler/ingress/controller.go index 47168f867..c5db4b65b 100644 --- a/pkg/reconciler/ingress/controller.go +++ b/pkg/reconciler/ingress/controller.go @@ -37,6 +37,7 @@ import ( gwapiclient "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/client" gatewayinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/gateway" httprouteinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/httproute" + referencepolicyinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencepolicy" "knative.dev/net-gateway-api/pkg/reconciler/ingress/config" ) @@ -55,12 +56,15 @@ func NewController( ingressInformer := ingressinformer.Get(ctx) httprouteInformer := httprouteinformer.Get(ctx) + referencePolicyInformer := referencepolicyinformer.Get(ctx) gatewayInformer := gatewayinformer.Get(ctx) endpointsInformer := endpointsinformer.Get(ctx) c := &Reconciler{ - gwapiclient: gwapiclient.Get(ctx), - httprouteLister: httprouteInformer.Lister(), + gwapiclient: gwapiclient.Get(ctx), + httprouteLister: httprouteInformer.Lister(), + referencePolicyLister: referencePolicyInformer.Lister(), + gatewayLister: gatewayInformer.Lister(), } filterFunc := reconciler.AnnotationFilterFunc(networking.IngressClassAnnotationKey, gatewayAPIIngressClassName, false) diff --git a/pkg/reconciler/ingress/controller_test.go b/pkg/reconciler/ingress/controller_test.go index 04ce042da..eebaad901 100644 --- a/pkg/reconciler/ingress/controller_test.go +++ b/pkg/reconciler/ingress/controller_test.go @@ -32,6 +32,7 @@ import ( _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/gateway/fake" _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/httproute/fake" + _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencepolicy/fake" "knative.dev/net-gateway-api/pkg/reconciler/ingress/config" ) diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index 2ad37f7b0..79252bbfd 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -49,6 +49,10 @@ type Reconciler struct { // Listers index properties about resources httprouteLister gatewayListers.HTTPRouteLister + + referencePolicyLister gatewayListers.ReferencePolicyLister + + gatewayLister gatewayListers.GatewayLister } var ( @@ -68,6 +72,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, ingress *v1alpha1.Ingres func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress) error { logger := logging.FromContext(ctx) + gatewayConfig := config.FromContext(ctx).Gateway // We may be reading a version of the object that was stored at an older version // and may not have had all of the assumed defaults specified. This won't result @@ -100,14 +105,33 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress logger.Infof("HTTPRoute successfully synced %v", httproutes) } + listeners := make([]*gatewayv1alpha2.Listener, 0, len(ing.Spec.TLS)) + for _, tls := range ing.Spec.TLS { + tls := tls + + l, err := c.reconcileTLS(ctx, &tls, ing) + if err != nil { + return err + } + listeners = append(listeners, l...) + } + + if len(listeners) > 0 { + // For now, we only reconcile the external visibility, because there's + // no way to provide TLS for internal listeners. + err := c.reconcileGatewayListeners( + ctx, listeners, ing, *gatewayConfig.Gateways[v1alpha1.IngressVisibilityExternalIP].Gateway) + if err != nil { + return err + } + } + ready, err := c.statusManager.IsReady(ctx, before) if err != nil { return fmt.Errorf("failed to probe Ingress: %w", err) } if ready { - gatewayConfig := config.FromContext(ctx).Gateway - namespacedNameService := gatewayConfig.Gateways[v1alpha1.IngressVisibilityExternalIP].Service publicLbs := []v1alpha1.LoadBalancerIngressStatus{ {DomainInternal: network.GetServiceHostname(namespacedNameService.Name, namespacedNameService.Namespace)}, diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index 370d67ba5..628247389 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" clientgotesting "k8s.io/client-go/testing" + "k8s.io/utils/pointer" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "knative.dev/networking/pkg/apis/networking" @@ -246,6 +247,99 @@ func TestReconcileProberNotReady(t *testing.T) { })) } +func TestReconcileTLS(t *testing.T) { + // The gateway API annoyingly has a number of + secretName := "name-WE-STICK-A-LONG-UID-HERE" + nsName := "ns" + myGw := gw(defaultListener) + table := TableTest{{ + Name: "Happy TLS", + Key: "ns/name", + SkipNamespaceValidation: true, // We need to create the Gateway in a different namespace + Objects: []runtime.Object{ + ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName)), + secret(secretName, nsName), + myGw, + }, + WantCreates: []runtime.Object{ + myGw, + httpRoute(t, ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName))), + rp(secret(secretName, nsName)), + }, + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: gw(defaultListener, func(g *gatewayv1alpha2.Gateway) { + g.Spec.Listeners = append(g.Spec.Listeners, gatewayv1alpha2.Listener{ + Name: "secure.example.com", + Hostname: (*gatewayv1alpha2.Hostname)(pointer.String("secure.example.com")), + Port: 443, + Protocol: "HTTPS", + TLS: &gatewayv1alpha2.GatewayTLSConfig{ + Mode: (*gatewayv1alpha2.TLSModeType)(pointer.String("Terminate")), + CertificateRefs: []*gatewayv1alpha2.SecretObjectReference{{ + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Secret")), + Name: gatewayv1alpha2.ObjectName(secretName), + Namespace: (*gatewayv1alpha2.Namespace)(&nsName), + }}, + Options: map[gatewayv1alpha2.AnnotationKey]gatewayv1alpha2.AnnotationValue{}, + }, + AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{ + Namespaces: &gatewayv1alpha2.RouteNamespaces{ + From: (*gatewayv1alpha2.FromNamespaces)(pointer.String("Selector")), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "kubernetes.io/metadata.name": "ns", + }, + }, + }, + Kinds: []gatewayv1alpha2.RouteGroupKind{}, + }, + }) + }), + }}, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName), func(i *v1alpha1.Ingress) { + i.Status.InitializeConditions() + i.Status.MarkLoadBalancerReady( + []v1alpha1.LoadBalancerIngressStatus{{ + DomainInternal: publicSvc, + }}, + []v1alpha1.LoadBalancerIngressStatus{{ + DomainInternal: privateSvc, + }}) + }), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Created", `Created HTTPRoute "example.com"`), + }, + }} + + table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { + r := &Reconciler{ + gwapiclient: fakegwapiclientset.Get(ctx), + httprouteLister: listers.GetHTTPRouteLister(), + referencePolicyLister: listers.GetReferencePolicyLister(), + gatewayLister: listers.GetGatewayLister(), + statusManager: &fakeStatusManager{FakeIsReady: func(context.Context, *v1alpha1.Ingress) (bool, error) { + return true, nil + }}, + } + + // The fake tracker's `Add` method incorrectly pluralizes "gatewaies" using UnsafeGuessKindToResource, + // so create this via explicit call (per note in client-go/testing/fixture.go in tracker.Add) + fakegwapiclientset.Get(ctx).GatewayV1alpha2().Gateways(myGw.Namespace).Create(ctx, myGw, metav1.CreateOptions{}) + + ingr := ingressreconciler.NewReconciler(ctx, logging.FromContext(ctx), fakeingressclient.Get(ctx), + listers.GetIngressLister(), controller.GetEventRecorder(ctx), r, gatewayAPIIngressClassName, + controller.Options{ + ConfigStore: &testConfigStore{ + config: defaultConfig, + }}) + + return ingr + })) +} + func TestReconcileProbeError(t *testing.T) { theError := errors.New("this is the error") @@ -336,6 +430,85 @@ func (t *testConfigStore) ToContext(ctx context.Context) context.Context { return config.ToContext(ctx, t.config) } +type GatewayOption func(*gatewayv1alpha2.Gateway) + +func gw(opts ...GatewayOption) *gatewayv1alpha2.Gateway { + g := &gatewayv1alpha2.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicName, + Namespace: testNamespace, + }, + Spec: gatewayv1alpha2.GatewaySpec{ + GatewayClassName: gatewayAPIIngressClassName, + }, + } + for _, opt := range opts { + opt(g) + } + return g +} + +func defaultListener(g *gatewayv1alpha2.Gateway) { + g.Spec.Listeners = append(g.Spec.Listeners, gatewayv1alpha2.Listener{ + Name: "http", + Port: 80, + Protocol: "HTTP", + }) +} + +func withTLS(secret string) IngressOption { + return func(i *v1alpha1.Ingress) { + i.Spec.TLS = append(i.Spec.TLS, v1alpha1.IngressTLS{ + Hosts: []string{"secure.example.com"}, + SecretName: "name-WE-STICK-A-LONG-UID-HERE", + SecretNamespace: "ns", + }) + } +} + +func secret(name, ns string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + }, + StringData: map[string]string{ + "ca.crt": "signed thing", + "ca.key": "private thing", + }, + Type: "kubernetes.io/tls", + } +} + +func rp(to *corev1.Secret) *gatewayv1alpha2.ReferencePolicy { + t := true + return &gatewayv1alpha2.ReferencePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: to.Name + "-" + testNamespace, + Namespace: to.Namespace, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "networking.internal.knative.dev/v1alpha1", + Kind: "Ingress", + Name: "name", + Controller: &t, + BlockOwnerDeletion: &t, + }}, + }, + Spec: gatewayv1alpha2.ReferencePolicySpec{ + From: []gatewayv1alpha2.ReferencePolicyFrom{{ + Group: "gateway.networking.k8s.io", + Kind: "Gateway", + Namespace: gatewayv1alpha2.Namespace(testNamespace), + }}, + To: []gatewayv1alpha2.ReferencePolicyTo{{ + Group: gatewayv1alpha2.Group(""), + Kind: gatewayv1alpha2.Kind("Secret"), + Name: (*gatewayv1alpha2.ObjectName)(&to.Name), + }}, + }, + } +} + var ( defaultConfig = &config.Config{ Network: &networkcfg.Config{}, diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index 0c152843d..359053b50 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -24,8 +24,11 @@ import ( "k8s.io/apimachinery/pkg/api/equality" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - gatewayv1alpa2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + "knative.dev/net-gateway-api/pkg/reconciler/ingress/config" "knative.dev/net-gateway-api/pkg/reconciler/ingress/resources" netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1" "knative.dev/pkg/controller" @@ -35,7 +38,7 @@ import ( func (c *Reconciler) reconcileHTTPRoute( ctx context.Context, ing *netv1alpha1.Ingress, rule *netv1alpha1.IngressRule, -) (*gatewayv1alpa2.HTTPRoute, error) { +) (*gatewayv1alpha2.HTTPRoute, error) { recorder := controller.GetEventRecorder(ctx) httproute, err := c.httprouteLister.HTTPRoutes(ing.Namespace).Get(resources.LongestHost(rule.Hosts)) @@ -82,3 +85,156 @@ func (c *Reconciler) reconcileHTTPRoute( return httproute, err } + +func (c *Reconciler) reconcileTLS( + ctx context.Context, tls *netv1alpha1.IngressTLS, ing *netv1alpha1.Ingress, +) ( + []*gatewayv1alpha2.Listener, error) { + recorder := controller.GetEventRecorder(ctx) + gatewayConfig := config.FromContext(ctx).Gateway.Gateways + externalGw := gatewayConfig[netv1alpha1.IngressVisibilityExternalIP] + + gateway := metav1.PartialObjectMetadata{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: gatewayv1alpha2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: externalGw.Gateway.Name, + Namespace: externalGw.Gateway.Namespace, + }, + } + secret := metav1.PartialObjectMetadata{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", //metav1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: tls.SecretName, + Namespace: tls.SecretNamespace, + }, + } + + desired := resources.MakeReferenceGrant(ctx, ing, secret, gateway) + + rp, err := c.referencePolicyLister.ReferencePolicies(desired.Namespace).Get(desired.Name) + + if apierrs.IsNotFound(err) { + rp, err = c.gwapiclient.GatewayV1alpha2().ReferencePolicies(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{}) + + if err != nil { + recorder.Eventf(ing, corev1.EventTypeWarning, "CreationFailed", "Failed to create ReferencePolicy: %v", err) + return nil, fmt.Errorf("failed to create ReferencePolicy: %w", err) + } + } else if err != nil { + return nil, err + } + + if !metav1.IsControlledBy(rp, ing) { + recorder.Eventf(ing, corev1.EventTypeWarning, "NotOwned", "ReferencePolicy %s not owned by this object", desired.Name) + return nil, fmt.Errorf("ReferencePolicy %s not owned by %s", rp.Name, ing.Name) + } + + if !equality.Semantic.DeepEqual(rp.Spec, desired.Spec) { + update := rp.DeepCopy() + update.Spec = desired.Spec + + _, err := c.gwapiclient.GatewayV1alpha2().ReferencePolicies(update.Namespace).Update(ctx, update, metav1.UpdateOptions{}) + if err != nil { + recorder.Eventf(ing, corev1.EventTypeWarning, "UpdateFailed", "Failed to update ReferencePolicy: %v", err) + return nil, fmt.Errorf("failed to update ReferencePolicy: %w", err) + } + } + + // Gateway API loves typed pointers and constants, so we need to copy the constants + // to something we can reference + mode := gatewayv1alpha2.TLSModeTerminate + selector := gatewayv1alpha2.NamespacesFromSelector + listeners := make([]*gatewayv1alpha2.Listener, 0, len(tls.Hosts)) + for _, h := range tls.Hosts { + listener := gatewayv1alpha2.Listener{ + Name: gatewayv1alpha2.SectionName(h), + Hostname: (*gatewayv1alpha2.Hostname)(&h), + Port: 443, + Protocol: gatewayv1alpha2.HTTPSProtocolType, + TLS: &gatewayv1alpha2.GatewayTLSConfig{ + Mode: &mode, + CertificateRefs: []*gatewayv1alpha2.SecretObjectReference{{ + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Secret")), + Name: gatewayv1alpha2.ObjectName(tls.SecretName), + Namespace: (*gatewayv1alpha2.Namespace)(&tls.SecretNamespace), + }}, + }, + AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{ + Namespaces: &gatewayv1alpha2.RouteNamespaces{ + From: &selector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + corev1.LabelMetadataName: ing.Namespace, + }, + }, + }, + Kinds: []gatewayv1alpha2.RouteGroupKind{}, + }, + } + listeners = append(listeners, &listener) + } + + return listeners, err +} + +func (c *Reconciler) reconcileGatewayListeners( + ctx context.Context, listeners []*gatewayv1alpha2.Listener, + ing *netv1alpha1.Ingress, gwName types.NamespacedName, +) error { + recorder := controller.GetEventRecorder(ctx) + gw, err := c.gatewayLister.Gateways(gwName.Namespace).Get(gwName.Name) + if apierrs.IsNotFound(err) { + recorder.Eventf(ing, corev1.EventTypeWarning, "GatewayMissing", "Unable to update Gateway %s", gwName.String()) + return fmt.Errorf("Gateway %s does not exist: %w", gwName, err) + } else if err != nil { + return err + } + + update := gw.DeepCopy() + + lmap := map[string]*gatewayv1alpha2.Listener{} + for _, l := range listeners { + lmap[string(l.Name)] = l + } + // TODO: how do we track and remove listeners if they are removed from the KIngress spec? + + updated := false + for i, l := range gw.Spec.Listeners { + desired, ok := lmap[string(l.Name)] + if !ok { + // This listener doesn't match any that we control. + continue + } + delete(lmap, string(l.Name)) + if equality.Semantic.DeepEqual(l, desired) { + // Already present and correct + continue + } + update.Spec.Listeners[i] = *desired + updated = true + } + + for _, l := range lmap { + // Add all remaining listeners + update.Spec.Listeners = append(update.Spec.Listeners, *l) + updated = true + } + + if updated { + _, err := c.gwapiclient.GatewayV1alpha2().Gateways(update.Namespace).Update( + ctx, update, metav1.UpdateOptions{}) + if err != nil { + recorder.Eventf(ing, corev1.EventTypeWarning, "GatewayUpdateFailed", "Failed to update Gateway %s: %v", gwName, err) + return fmt.Errorf("failed to update Gateway %s/%s: %w", update.Namespace, update.Name, err) + } + } + + return nil +} diff --git a/pkg/reconciler/ingress/resources/httproute.go b/pkg/reconciler/ingress/resources/httproute.go index 46f30a738..788909a70 100644 --- a/pkg/reconciler/ingress/resources/httproute.go +++ b/pkg/reconciler/ingress/resources/httproute.go @@ -23,7 +23,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" - gatewayv1alpa2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "knative.dev/net-gateway-api/pkg/reconciler/ingress/config" "knative.dev/networking/pkg/apis/networking" @@ -36,14 +36,14 @@ func MakeHTTPRoute( ctx context.Context, ing *netv1alpha1.Ingress, rule *netv1alpha1.IngressRule, -) (*gatewayv1alpa2.HTTPRoute, error) { +) (*gatewayv1alpha2.HTTPRoute, error) { visibility := "" if rule.Visibility == netv1alpha1.IngressVisibilityClusterLocal { visibility = "cluster-local" } - return &gatewayv1alpa2.HTTPRoute{ + return &gatewayv1alpha2.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: LongestHost(rule.Hosts), Namespace: ing.Namespace, @@ -62,11 +62,11 @@ func MakeHTTPRoute( func makeHTTPRouteSpec( ctx context.Context, rule *netv1alpha1.IngressRule, -) gatewayv1alpa2.HTTPRouteSpec { +) gatewayv1alpha2.HTTPRouteSpec { - hostnames := make([]gatewayv1alpa2.Hostname, 0, len(rule.Hosts)) + hostnames := make([]gatewayv1alpha2.Hostname, 0, len(rule.Hosts)) for _, hostname := range rule.Hosts { - hostnames = append(hostnames, gatewayv1alpa2.Hostname(hostname)) + hostnames = append(hostnames, gatewayv1alpha2.Hostname(hostname)) } rules := makeHTTPRouteRule(rule) @@ -74,32 +74,32 @@ func makeHTTPRouteSpec( gatewayConfig := config.FromContext(ctx).Gateway namespacedNameGateway := gatewayConfig.Gateways[rule.Visibility].Gateway - gatewayRef := gatewayv1alpa2.ParentRef{ - Namespace: namespacePtr(gatewayv1alpa2.Namespace(namespacedNameGateway.Namespace)), - Name: gatewayv1alpa2.ObjectName(namespacedNameGateway.Name), + gatewayRef := gatewayv1alpha2.ParentRef{ + Namespace: namespacePtr(gatewayv1alpha2.Namespace(namespacedNameGateway.Namespace)), + Name: gatewayv1alpha2.ObjectName(namespacedNameGateway.Name), } - return gatewayv1alpa2.HTTPRouteSpec{ + return gatewayv1alpha2.HTTPRouteSpec{ Hostnames: hostnames, Rules: rules, - CommonRouteSpec: gatewayv1alpa2.CommonRouteSpec{ParentRefs: []gatewayv1alpa2.ParentRef{ + CommonRouteSpec: gatewayv1alpha2.CommonRouteSpec{ParentRefs: []gatewayv1alpha2.ParentRef{ gatewayRef, }}, } } -func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayv1alpa2.HTTPRouteRule { - rules := []gatewayv1alpa2.HTTPRouteRule{} +func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayv1alpha2.HTTPRouteRule { + rules := []gatewayv1alpha2.HTTPRouteRule{} for _, path := range rule.HTTP.Paths { - backendRefs := make([]gatewayv1alpa2.HTTPBackendRef, 0, len(path.Splits)) - var preFilters []gatewayv1alpa2.HTTPRouteFilter + backendRefs := make([]gatewayv1alpha2.HTTPBackendRef, 0, len(path.Splits)) + var preFilters []gatewayv1alpha2.HTTPRouteFilter if path.AppendHeaders != nil { - headers := []gatewayv1alpa2.HTTPHeader{} + headers := []gatewayv1alpha2.HTTPHeader{} for k, v := range path.AppendHeaders { - header := gatewayv1alpa2.HTTPHeader{ - Name: gatewayv1alpa2.HTTPHeaderName(k), + header := gatewayv1alpha2.HTTPHeader{ + Name: gatewayv1alpha2.HTTPHeaderName(k), Value: v, } headers = append(headers, header) @@ -108,18 +108,18 @@ func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayv1alpa2.HTTPRoute // Sort HTTPHeader as the order is random. sort.Sort(HTTPHeaderList(headers)) - preFilters = []gatewayv1alpa2.HTTPRouteFilter{{ - Type: gatewayv1alpa2.HTTPRouteFilterRequestHeaderModifier, - RequestHeaderModifier: &gatewayv1alpa2.HTTPRequestHeaderFilter{ + preFilters = []gatewayv1alpha2.HTTPRouteFilter{{ + Type: gatewayv1alpha2.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1alpha2.HTTPRequestHeaderFilter{ Set: headers, }}} } for _, split := range path.Splits { - headers := []gatewayv1alpa2.HTTPHeader{} + headers := []gatewayv1alpha2.HTTPHeader{} for k, v := range split.AppendHeaders { - header := gatewayv1alpa2.HTTPHeader{ - Name: gatewayv1alpa2.HTTPHeaderName(k), + header := gatewayv1alpha2.HTTPHeader{ + Name: gatewayv1alpha2.HTTPHeaderName(k), Value: v, } headers = append(headers, header) @@ -129,17 +129,17 @@ func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayv1alpa2.HTTPRoute sort.Sort(HTTPHeaderList(headers)) name := split.IngressBackend.ServiceName - backendRef := gatewayv1alpa2.HTTPBackendRef{ - BackendRef: gatewayv1alpa2.BackendRef{ - BackendObjectReference: gatewayv1alpa2.BackendObjectReference{ + backendRef := gatewayv1alpha2.HTTPBackendRef{ + BackendRef: gatewayv1alpha2.BackendRef{ + BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Port: portNumPtr(split.ServicePort.IntValue()), - Name: gatewayv1alpa2.ObjectName(name), + Name: gatewayv1alpha2.ObjectName(name), }, Weight: pointer.Int32Ptr(int32(split.Percent)), }, - Filters: []gatewayv1alpa2.HTTPRouteFilter{{ - Type: gatewayv1alpa2.HTTPRouteFilterRequestHeaderModifier, - RequestHeaderModifier: &gatewayv1alpa2.HTTPRequestHeaderFilter{ + Filters: []gatewayv1alpha2.HTTPRouteFilter{{ + Type: gatewayv1alpha2.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1alpha2.HTTPRequestHeaderFilter{ Set: headers, }}, }} @@ -150,16 +150,16 @@ func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayv1alpa2.HTTPRoute if path.Path != "" { pathPrefix = path.Path } - pathMatch := gatewayv1alpa2.HTTPPathMatch{ - Type: pathMatchTypePtr(gatewayv1alpa2.PathMatchPathPrefix), + pathMatch := gatewayv1alpha2.HTTPPathMatch{ + Type: pathMatchTypePtr(gatewayv1alpha2.PathMatchPathPrefix), Value: pointer.StringPtr(pathPrefix), } - headerMatchList := []gatewayv1alpa2.HTTPHeaderMatch{} + headerMatchList := []gatewayv1alpha2.HTTPHeaderMatch{} for k, v := range path.Headers { - headerMatch := gatewayv1alpa2.HTTPHeaderMatch{ - Type: headerMatchTypePtr(gatewayv1alpa2.HeaderMatchExact), - Name: gatewayv1alpa2.HTTPHeaderName(k), + headerMatch := gatewayv1alpha2.HTTPHeaderMatch{ + Type: headerMatchTypePtr(gatewayv1alpha2.HeaderMatchExact), + Name: gatewayv1alpha2.HTTPHeaderName(k), Value: v.Exact, } headerMatchList = append(headerMatchList, headerMatch) @@ -168,9 +168,9 @@ func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayv1alpa2.HTTPRoute // Sort HTTPHeaderMatch as the order is random. sort.Sort(HTTPHeaderMatchList(headerMatchList)) - matches := []gatewayv1alpa2.HTTPRouteMatch{{Path: &pathMatch, Headers: headerMatchList}} + matches := []gatewayv1alpha2.HTTPRouteMatch{{Path: &pathMatch, Headers: headerMatchList}} - rule := gatewayv1alpa2.HTTPRouteRule{ + rule := gatewayv1alpha2.HTTPRouteRule{ BackendRefs: backendRefs, Filters: preFilters, Matches: matches, @@ -180,7 +180,7 @@ func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayv1alpa2.HTTPRoute return rules } -type HTTPHeaderList []gatewayv1alpa2.HTTPHeader +type HTTPHeaderList []gatewayv1alpha2.HTTPHeader func (h HTTPHeaderList) Len() int { return len(h) @@ -194,7 +194,7 @@ func (h HTTPHeaderList) Swap(i, j int) { h[i], h[j] = h[j], h[i] } -type HTTPHeaderMatchList []gatewayv1alpa2.HTTPHeaderMatch +type HTTPHeaderMatchList []gatewayv1alpha2.HTTPHeaderMatch func (h HTTPHeaderMatchList) Len() int { return len(h) diff --git a/pkg/reconciler/ingress/resources/reference_grant.go b/pkg/reconciler/ingress/resources/reference_grant.go new file mode 100644 index 000000000..726fe8373 --- /dev/null +++ b/pkg/reconciler/ingress/resources/reference_grant.go @@ -0,0 +1,60 @@ +/* +Copyright 2021 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed from in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1" + "knative.dev/pkg/kmeta" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +// Grant the resource "to" access to the resource "from" +// N.B. ReferencePolicy will be renamed ReferenceGrant in v1beta1 +func MakeReferenceGrant(ctx context.Context, ing *netv1alpha1.Ingress, to, from metav1.PartialObjectMetadata) *gatewayv1alpha2.ReferencePolicy { + name := to.Name + if len(name)+len(from.Namespace) > 62 { + name = name[:62-len(from.Namespace)] + } + name += "-" + from.Namespace + + retval := &gatewayv1alpha2.ReferencePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: to.Namespace, + Labels: to.Labels, + Annotations: to.Annotations, + OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)}, + }, + Spec: gatewayv1alpha2.ReferencePolicySpec{ + From: []gatewayv1alpha2.ReferencePolicyFrom{{ + Group: gatewayv1alpha2.Group(from.GroupVersionKind().Group), + Kind: gatewayv1alpha2.Kind(from.Kind), + Namespace: gatewayv1alpha2.Namespace(from.Namespace), + }}, + To: []gatewayv1alpha2.ReferencePolicyTo{{ + Group: gatewayv1alpha2.Group(to.GroupVersionKind().Group), + Kind: gatewayv1alpha2.Kind(to.Kind), + Name: (*gatewayv1alpha2.ObjectName)(&to.Name), + }}, + }, + } + + return retval +} diff --git a/pkg/reconciler/testing/listers.go b/pkg/reconciler/testing/listers.go index 5c799c6c4..bfe963f12 100644 --- a/pkg/reconciler/testing/listers.go +++ b/pkg/reconciler/testing/listers.go @@ -99,3 +99,11 @@ func (l *Listers) GetHTTPRouteLister() gatewayListers.HTTPRouteLister { func (l *Listers) GetEndpointsLister() corev1listers.EndpointsLister { return corev1listers.NewEndpointsLister(l.IndexerFor(&corev1.Endpoints{})) } + +func (l *Listers) GetGatewayLister() gatewayListers.GatewayLister { + return gatewayListers.NewGatewayLister(l.IndexerFor(&gatewayv1alpa2.Gateway{})) +} + +func (l *Listers) GetReferencePolicyLister() gatewayListers.ReferencePolicyLister { + return gatewayListers.NewReferencePolicyLister(l.IndexerFor(&gatewayv1alpa2.ReferencePolicy{})) +} From 811acab2d362b3b142a87087f1200becb86859d6 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 28 Jun 2022 13:48:01 -0400 Subject: [PATCH 02/10] Fix codegen update and run --- docs/test-version.md | 4 ++-- hack/test-env.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/test-version.md b/docs/test-version.md index 1dac2ef30..358bbd2b3 100644 --- a/docs/test-version.md +++ b/docs/test-version.md @@ -15,5 +15,5 @@ The following Gateway API version and Ingress were tested as part of the release | Ingress | Tested version | Unavailable features | | ------- | ----------------------- | ------------------------------ | -| Istio | v1.13.2 | tls,retry,httpoption,host-rewrite | -| Contour | v1.21.0 | tls,retry,httpoption,basics/http2,websocket,websocket/split,grpc,grpc/split,visibility/path,visibility,update,host-rewrite | +| Istio | v1.13.2 | retry,httpoption,host-rewrite | +| Contour | v1.21.0 | retry,httpoption,basics/http2,websocket,websocket/split,grpc,grpc/split,visibility/path,visibility,update,host-rewrite | diff --git a/hack/test-env.sh b/hack/test-env.sh index c1714548f..82194b247 100755 --- a/hack/test-env.sh +++ b/hack/test-env.sh @@ -16,6 +16,6 @@ export GATEWAY_API_VERSION="v0.4.3" export ISTIO_VERSION="1.13.2" -export ISTIO_UNSUPPORTED_TESTS="retry,httpoption,host-rewrite" +export ISTIO_UNSUPPORTED_E2E_TESTS="retry,httpoption,host-rewrite" export CONTOUR_VERSION="v1.21.0" -export CONTOUR_UNSUPPORTED_TESTS="retry,httpoption,basics/http2,websocket,websocket/split,grpc,grpc/split,visibility/path,visibility,update,host-rewrite" +export CONTOUR_UNSUPPORTED_E2E_TESTS="retry,httpoption,basics/http2,websocket,websocket/split,grpc,grpc/split,visibility/path,visibility,update,host-rewrite" From aba67bdf085e26dc01e36132bbb6831d5a1e6b16 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 28 Jun 2022 17:46:01 -0400 Subject: [PATCH 03/10] Fix reconcile on no changes; add reconciliation tests --- pkg/reconciler/ingress/ingress_test.go | 141 ++++++++++++++---- pkg/reconciler/ingress/reconcile_resources.go | 2 +- 2 files changed, 111 insertions(+), 32 deletions(-) diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index 628247389..241db7956 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -267,36 +267,40 @@ func TestReconcileTLS(t *testing.T) { rp(secret(secretName, nsName)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: gw(defaultListener, func(g *gatewayv1alpha2.Gateway) { - g.Spec.Listeners = append(g.Spec.Listeners, gatewayv1alpha2.Listener{ - Name: "secure.example.com", - Hostname: (*gatewayv1alpha2.Hostname)(pointer.String("secure.example.com")), - Port: 443, - Protocol: "HTTPS", - TLS: &gatewayv1alpha2.GatewayTLSConfig{ - Mode: (*gatewayv1alpha2.TLSModeType)(pointer.String("Terminate")), - CertificateRefs: []*gatewayv1alpha2.SecretObjectReference{{ - Group: (*gatewayv1alpha2.Group)(pointer.String("")), - Kind: (*gatewayv1alpha2.Kind)(pointer.String("Secret")), - Name: gatewayv1alpha2.ObjectName(secretName), - Namespace: (*gatewayv1alpha2.Namespace)(&nsName), - }}, - Options: map[gatewayv1alpha2.AnnotationKey]gatewayv1alpha2.AnnotationValue{}, - }, - AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{ - Namespaces: &gatewayv1alpha2.RouteNamespaces{ - From: (*gatewayv1alpha2.FromNamespaces)(pointer.String("Selector")), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "kubernetes.io/metadata.name": "ns", - }, - }, - }, - Kinds: []gatewayv1alpha2.RouteGroupKind{}, - }, - }) + Object: gw(defaultListener, tlsListener("secure.example.com", nsName, secretName)), + }}, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName), func(i *v1alpha1.Ingress) { + i.Status.InitializeConditions() + i.Status.MarkLoadBalancerReady( + []v1alpha1.LoadBalancerIngressStatus{{ + DomainInternal: publicSvc, + }}, + []v1alpha1.LoadBalancerIngressStatus{{ + DomainInternal: privateSvc, + }}) }), }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Created", `Created HTTPRoute "example.com"`), + }, + }, { + Name: "Already Configured", + Key: "ns/name", + SkipNamespaceValidation: true, + Objects: []runtime.Object{ + ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName)), + secret(secretName, nsName), + gw(defaultListener, tlsListener("secure.example.com", nsName, secretName)), + httpRoute(t, ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName))), + rp(secret(secretName, nsName)), + }, + WantCreates: []runtime.Object{ + gw(defaultListener, tlsListener("secure.example.com", nsName, secretName)), + }, + WantUpdates: []clientgotesting.UpdateActionImpl{ + // None + }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName), func(i *v1alpha1.Ingress) { i.Status.InitializeConditions() @@ -309,12 +313,39 @@ func TestReconcileTLS(t *testing.T) { }}) }), }}, + WantEvents: []string{ + // None + }, + }, { + Name: "No Gateway", + Key: "ns/name", + SkipNamespaceValidation: true, + WantErr: true, + Objects: []runtime.Object{ + ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName)), + secret(secretName, nsName), + }, + WantCreates: []runtime.Object{ + httpRoute(t, ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName))), + rp(secret(secretName, nsName)), + }, + WantUpdates: []clientgotesting.UpdateActionImpl{ + // None + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName), func(i *v1alpha1.Ingress) { + i.Status.InitializeConditions() + i.Status.MarkIngressNotReady("ReconcileIngressFailed", "Ingress reconciliation failed") + }), + }}, WantEvents: []string{ Eventf(corev1.EventTypeNormal, "Created", `Created HTTPRoute "example.com"`), + Eventf(corev1.EventTypeWarning, "GatewayMissing", `Unable to update Gateway istio-system/istio-gateway`), + Eventf(corev1.EventTypeWarning, "InternalError", `Gateway istio-system/istio-gateway does not exist: gateway.gateway.networking.k8s.io "istio-gateway" not found`), }, }} - table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { + table.Test(t, GatewayFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher, tr *TableRow) controller.Reconciler { r := &Reconciler{ gwapiclient: fakegwapiclientset.Get(ctx), httprouteLister: listers.GetHTTPRouteLister(), @@ -324,10 +355,14 @@ func TestReconcileTLS(t *testing.T) { return true, nil }}, } - // The fake tracker's `Add` method incorrectly pluralizes "gatewaies" using UnsafeGuessKindToResource, // so create this via explicit call (per note in client-go/testing/fixture.go in tracker.Add) - fakegwapiclientset.Get(ctx).GatewayV1alpha2().Gateways(myGw.Namespace).Create(ctx, myGw, metav1.CreateOptions{}) + for _, x := range tr.Objects { + myGw, ok := x.(*gatewayv1alpha2.Gateway) + if ok { + fakegwapiclientset.Get(ctx).GatewayV1alpha2().Gateways(myGw.Namespace).Create(ctx, myGw, metav1.CreateOptions{}) + } + } ingr := ingressreconciler.NewReconciler(ctx, logging.FromContext(ctx), fakeingressclient.Get(ctx), listers.GetIngressLister(), controller.GetEventRecorder(ctx), r, gatewayAPIIngressClassName, @@ -430,6 +465,18 @@ func (t *testConfigStore) ToContext(ctx context.Context) context.Context { return config.ToContext(ctx, t.config) } +// We need to inject the row's `Objects` to work-around improper pluralization in UnsafeGuessKindToResource +func GatewayFactory(ctor func(context.Context, *Listers, configmap.Watcher, *TableRow) controller.Reconciler) Factory { + return func(t *testing.T, r *TableRow) ( + controller.Reconciler, ActionRecorderList, EventList, + ) { + shim := func(c context.Context, l *Listers, cw configmap.Watcher) controller.Reconciler { + return ctor(c, l, cw, r) + } + return MakeFactory(shim)(t, r) + } +} + type GatewayOption func(*gatewayv1alpha2.Gateway) func gw(opts ...GatewayOption) *gatewayv1alpha2.Gateway { @@ -456,6 +503,38 @@ func defaultListener(g *gatewayv1alpha2.Gateway) { }) } +func tlsListener(hostname, nsName, secretName string) GatewayOption { + return func(g *gatewayv1alpha2.Gateway) { + g.Spec.Listeners = append(g.Spec.Listeners, gatewayv1alpha2.Listener{ + Name: gatewayv1alpha2.SectionName(hostname), + Hostname: (*gatewayv1alpha2.Hostname)(&hostname), + Port: 443, + Protocol: "HTTPS", + TLS: &gatewayv1alpha2.GatewayTLSConfig{ + Mode: (*gatewayv1alpha2.TLSModeType)(pointer.String("Terminate")), + CertificateRefs: []*gatewayv1alpha2.SecretObjectReference{{ + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Secret")), + Name: gatewayv1alpha2.ObjectName(secretName), + Namespace: (*gatewayv1alpha2.Namespace)(&nsName), + }}, + Options: map[gatewayv1alpha2.AnnotationKey]gatewayv1alpha2.AnnotationValue{}, + }, + AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{ + Namespaces: &gatewayv1alpha2.RouteNamespaces{ + From: (*gatewayv1alpha2.FromNamespaces)(pointer.String("Selector")), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "kubernetes.io/metadata.name": nsName, + }, + }, + }, + Kinds: []gatewayv1alpha2.RouteGroupKind{}, + }, + }) + } +} + func withTLS(secret string) IngressOption { return func(i *v1alpha1.Ingress) { i.Spec.TLS = append(i.Spec.TLS, v1alpha1.IngressTLS{ diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index 359053b50..803774b34 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -213,7 +213,7 @@ func (c *Reconciler) reconcileGatewayListeners( continue } delete(lmap, string(l.Name)) - if equality.Semantic.DeepEqual(l, desired) { + if equality.Semantic.DeepEqual(&l, desired) { // Already present and correct continue } From d986f35bb7a71c4704b96c0d6e400528c900df85 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 28 Jun 2022 20:12:37 -0400 Subject: [PATCH 04/10] Update with Dave's feedback. --- pkg/reconciler/ingress/reconcile_resources.go | 4 +++- pkg/reconciler/ingress/resources/reference_grant.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index 803774b34..be0e765be 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -107,7 +107,7 @@ func (c *Reconciler) reconcileTLS( secret := metav1.PartialObjectMetadata{ TypeMeta: metav1.TypeMeta{ Kind: "Secret", - APIVersion: "v1", //metav1.GroupVersion.String(), + APIVersion: corev1.SchemeGroupVersion.Version, }, ObjectMeta: metav1.ObjectMeta{ Name: tls.SecretName, @@ -152,6 +152,7 @@ func (c *Reconciler) reconcileTLS( selector := gatewayv1alpha2.NamespacesFromSelector listeners := make([]*gatewayv1alpha2.Listener, 0, len(tls.Hosts)) for _, h := range tls.Hosts { + h := h listener := gatewayv1alpha2.Listener{ Name: gatewayv1alpha2.SectionName(h), Hostname: (*gatewayv1alpha2.Hostname)(&h), @@ -204,6 +205,7 @@ func (c *Reconciler) reconcileGatewayListeners( lmap[string(l.Name)] = l } // TODO: how do we track and remove listeners if they are removed from the KIngress spec? + // Tracked in https://github.com/knative-sandbox/net-gateway-api/issues/319 updated := false for i, l := range gw.Spec.Listeners { diff --git a/pkg/reconciler/ingress/resources/reference_grant.go b/pkg/reconciler/ingress/resources/reference_grant.go index 726fe8373..42172fff6 100644 --- a/pkg/reconciler/ingress/resources/reference_grant.go +++ b/pkg/reconciler/ingress/resources/reference_grant.go @@ -7,7 +7,7 @@ You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed from in writing, software +Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and From 2cdb1e85efc119e4764f355f6c621357cb152743 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Wed, 29 Jun 2022 02:05:24 -0400 Subject: [PATCH 05/10] Fix lint warnings. --- pkg/reconciler/ingress/reconcile_resources.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index be0e765be..7d4d7a438 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -193,7 +193,7 @@ func (c *Reconciler) reconcileGatewayListeners( gw, err := c.gatewayLister.Gateways(gwName.Namespace).Get(gwName.Name) if apierrs.IsNotFound(err) { recorder.Eventf(ing, corev1.EventTypeWarning, "GatewayMissing", "Unable to update Gateway %s", gwName.String()) - return fmt.Errorf("Gateway %s does not exist: %w", gwName, err) + return fmt.Errorf("Gateway %s does not exist: %w", gwName, err) //nolint:stylecheck } else if err != nil { return err } @@ -209,6 +209,7 @@ func (c *Reconciler) reconcileGatewayListeners( updated := false for i, l := range gw.Spec.Listeners { + l := l desired, ok := lmap[string(l.Name)] if !ok { // This listener doesn't match any that we control. From 1592176e213c0a0ae0f1bf1ea3e7eed174364c98 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 5 Jul 2022 20:08:21 -0400 Subject: [PATCH 06/10] Add Finalizer to cleanup Gateway Listeners Added some (defaulted) fields to HTTPRoute to avoid controller looping. --- pkg/reconciler/ingress/ingress.go | 16 ++- pkg/reconciler/ingress/ingress_test.go | 99 +++++++++++++++---- pkg/reconciler/ingress/reconcile_resources.go | 40 +++++++- pkg/reconciler/ingress/resources/httproute.go | 10 +- .../ingress/resources/httproute_test.go | 44 ++++++--- 5 files changed, 168 insertions(+), 41 deletions(-) diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index 79252bbfd..aab3803bb 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -27,7 +27,6 @@ import ( ingressreconciler "knative.dev/networking/pkg/client/injection/reconciler/networking/v1alpha1/ingress" "knative.dev/networking/pkg/ingress" "knative.dev/networking/pkg/status" - "knative.dev/pkg/logging" "knative.dev/pkg/network" pkgreconciler "knative.dev/pkg/reconciler" @@ -62,6 +61,7 @@ var ( // ReconcileKind implements Interface.ReconcileKind. func (c *Reconciler) ReconcileKind(ctx context.Context, ingress *v1alpha1.Ingress) pkgreconciler.Event { reconcileErr := c.reconcileIngress(ctx, ingress) + if reconcileErr != nil { ingress.Status.MarkIngressNotReady(notReconciledReason, notReconciledMessage) return reconcileErr @@ -70,8 +70,15 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, ingress *v1alpha1.Ingres return nil } +// FinalizeKind implements Interface.FinalizeKind +func (c *Reconciler) FinalizeKind(ctx context.Context, ingress *v1alpha1.Ingress) pkgreconciler.Event { + gatewayConfig := config.FromContext(ctx).Gateway + + // We currently only support TLS on the external IP + return c.clearGatewayListeners(ctx, ingress, gatewayConfig.Gateways[v1alpha1.IngressVisibilityExternalIP].Gateway) +} + func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress) error { - logger := logging.FromContext(ctx) gatewayConfig := config.FromContext(ctx).Gateway // We may be reading a version of the object that was stored at an older version @@ -87,8 +94,6 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress return fmt.Errorf("failed to add knative probe header: %w", err) } - logger.Infof("Reconciling ingress: %#v", ing) - for _, rule := range ing.Spec.Rules { rule := rule @@ -102,7 +107,6 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress } else { ing.Status.MarkIngressNotReady("HTTPRouteNotReady", "Waiting for HTTPRoute becomes Ready.") } - logger.Infof("HTTPRoute successfully synced %v", httproutes) } listeners := make([]*gatewayv1alpha2.Listener, 0, len(ing.Spec.TLS)) @@ -126,6 +130,8 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress } } + // TODO: check Gateway readiness before reporting Ingress ready + ready, err := c.statusManager.IsReady(ctx, before) if err != nil { return fmt.Errorf("failed to probe Ingress: %w", err) diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index 241db7956..fb86a1b18 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -172,14 +172,22 @@ func TestReconcile(t *testing.T) { }}) }), }}, + WantPatches: []clientgotesting.PatchActionImpl{{ + ActionImpl: clientgotesting.ActionImpl{ + Namespace: "ns", + }, + Name: "name", + Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), + }}, WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com\""), }, }, { Name: "reconcile ready ingress", Key: "ns/name", Objects: append([]runtime.Object{ - ing(withBasicSpec, withGatewayAPIclass, makeItReady), + ing(withBasicSpec, withGatewayAPIclass, makeItReady, withFinalizer), httpRoute(t, ing(withBasicSpec, withGatewayAPIclass)), }, servicesAndEndpoints...), // no extra update @@ -190,6 +198,7 @@ func TestReconcile(t *testing.T) { gwapiclient: fakegwapiclientset.Get(ctx), // Listers index properties about resources httprouteLister: listers.GetHTTPRouteLister(), + gatewayLister: listers.GetGatewayLister(), statusManager: &fakeStatusManager{ FakeIsReady: func(context.Context, *v1alpha1.Ingress) (bool, error) { return true, nil @@ -222,7 +231,15 @@ func TestReconcileProberNotReady(t *testing.T) { i.Status.MarkLoadBalancerNotReady() }), }}, + WantPatches: []clientgotesting.PatchActionImpl{{ + ActionImpl: clientgotesting.ActionImpl{ + Namespace: "ns", + }, + Name: "name", + Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), + }}, WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com\""), }, }} @@ -232,6 +249,7 @@ func TestReconcileProberNotReady(t *testing.T) { gwapiclient: fakegwapiclientset.Get(ctx), // Listers index properties about resources httprouteLister: listers.GetHTTPRouteLister(), + gatewayLister: listers.GetGatewayLister(), statusManager: &fakeStatusManager{ FakeIsReady: func(context.Context, *v1alpha1.Ingress) (bool, error) { return false, nil @@ -251,24 +269,29 @@ func TestReconcileTLS(t *testing.T) { // The gateway API annoyingly has a number of secretName := "name-WE-STICK-A-LONG-UID-HERE" nsName := "ns" - myGw := gw(defaultListener) + deleteTime := time.Now().Add(-10 * time.Second) table := TableTest{{ - Name: "Happy TLS", - Key: "ns/name", - SkipNamespaceValidation: true, // We need to create the Gateway in a different namespace + Name: "Happy TLS", + Key: "ns/name", Objects: []runtime.Object{ ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName)), secret(secretName, nsName), - myGw, + gw(defaultListener), }, WantCreates: []runtime.Object{ - myGw, httpRoute(t, ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName))), rp(secret(secretName, nsName)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: gw(defaultListener, tlsListener("secure.example.com", nsName, secretName)), }}, + WantPatches: []clientgotesting.PatchActionImpl{{ + ActionImpl: clientgotesting.ActionImpl{ + Namespace: "ns", + }, + Name: "name", + Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), + }}, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName), func(i *v1alpha1.Ingress) { i.Status.InitializeConditions() @@ -282,27 +305,24 @@ func TestReconcileTLS(t *testing.T) { }), }}, WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), Eventf(corev1.EventTypeNormal, "Created", `Created HTTPRoute "example.com"`), }, }, { - Name: "Already Configured", - Key: "ns/name", - SkipNamespaceValidation: true, + Name: "Already Configured", + Key: "ns/name", Objects: []runtime.Object{ - ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName)), + ing(withBasicSpec, withFinalizer, withGatewayAPIClass, withTLS(secretName)), secret(secretName, nsName), gw(defaultListener, tlsListener("secure.example.com", nsName, secretName)), httpRoute(t, ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName))), rp(secret(secretName, nsName)), }, - WantCreates: []runtime.Object{ - gw(defaultListener, tlsListener("secure.example.com", nsName, secretName)), - }, WantUpdates: []clientgotesting.UpdateActionImpl{ // None }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName), func(i *v1alpha1.Ingress) { + Object: ing(withBasicSpec, withFinalizer, withGatewayAPIClass, withTLS(secretName), func(i *v1alpha1.Ingress) { i.Status.InitializeConditions() i.Status.MarkLoadBalancerReady( []v1alpha1.LoadBalancerIngressStatus{{ @@ -317,10 +337,27 @@ func TestReconcileTLS(t *testing.T) { // None }, }, { - Name: "No Gateway", + Name: "Cleanup Listener", Key: "ns/name", SkipNamespaceValidation: true, - WantErr: true, + Objects: []runtime.Object{ + ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName), func(i *v1alpha1.Ingress) { + i.DeletionTimestamp = &metav1.Time{ + Time: deleteTime, + } + }), + secret(secretName, nsName), + gw(defaultListener, tlsListener("secure.example.com", nsName, secretName)), + httpRoute(t, ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName))), + rp(secret(secretName, nsName)), + }, + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: gw(defaultListener), + }}, + }, { + Name: "No Gateway", + Key: "ns/name", + WantErr: true, Objects: []runtime.Object{ ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName)), secret(secretName, nsName), @@ -332,6 +369,13 @@ func TestReconcileTLS(t *testing.T) { WantUpdates: []clientgotesting.UpdateActionImpl{ // None }, + WantPatches: []clientgotesting.PatchActionImpl{{ + ActionImpl: clientgotesting.ActionImpl{ + Namespace: "ns", + }, + Name: "name", + Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), + }}, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: ing(withBasicSpec, withGatewayAPIClass, withTLS(secretName), func(i *v1alpha1.Ingress) { i.Status.InitializeConditions() @@ -339,6 +383,7 @@ func TestReconcileTLS(t *testing.T) { }), }}, WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), Eventf(corev1.EventTypeNormal, "Created", `Created HTTPRoute "example.com"`), Eventf(corev1.EventTypeWarning, "GatewayMissing", `Unable to update Gateway istio-system/istio-gateway`), Eventf(corev1.EventTypeWarning, "InternalError", `Gateway istio-system/istio-gateway does not exist: gateway.gateway.networking.k8s.io "istio-gateway" not found`), @@ -357,12 +402,16 @@ func TestReconcileTLS(t *testing.T) { } // The fake tracker's `Add` method incorrectly pluralizes "gatewaies" using UnsafeGuessKindToResource, // so create this via explicit call (per note in client-go/testing/fixture.go in tracker.Add) + fakeCreates := []runtime.Object{} for _, x := range tr.Objects { myGw, ok := x.(*gatewayv1alpha2.Gateway) if ok { fakegwapiclientset.Get(ctx).GatewayV1alpha2().Gateways(myGw.Namespace).Create(ctx, myGw, metav1.CreateOptions{}) + tr.SkipNamespaceValidation = true + fakeCreates = append(fakeCreates, myGw) } } + tr.WantCreates = append(fakeCreates, tr.WantCreates...) ingr := ingressreconciler.NewReconciler(ctx, logging.FromContext(ctx), fakeingressclient.Get(ctx), listers.GetIngressLister(), controller.GetEventRecorder(ctx), r, gatewayAPIIngressClassName, @@ -392,7 +441,15 @@ func TestReconcileProbeError(t *testing.T) { i.Status.MarkIngressNotReady(notReconciledReason, notReconciledMessage) }), }}, + WantPatches: []clientgotesting.PatchActionImpl{{ + ActionImpl: clientgotesting.ActionImpl{ + Namespace: "ns", + }, + Name: "name", + Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), + }}, WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com\""), Eventf(corev1.EventTypeWarning, "InternalError", fmt.Sprintf("failed to probe Ingress: %v", theError)), }, @@ -403,6 +460,8 @@ func TestReconcileProbeError(t *testing.T) { gwapiclient: fakegwapiclientset.Get(ctx), // Listers index properties about resources httprouteLister: listers.GetHTTPRouteLister(), + gatewayLister: listers.GetGatewayLister(), + statusManager: &fakeStatusManager{ FakeIsReady: func(context.Context, *v1alpha1.Ingress) (bool, error) { return false, theError @@ -506,7 +565,7 @@ func defaultListener(g *gatewayv1alpha2.Gateway) { func tlsListener(hostname, nsName, secretName string) GatewayOption { return func(g *gatewayv1alpha2.Gateway) { g.Spec.Listeners = append(g.Spec.Listeners, gatewayv1alpha2.Listener{ - Name: gatewayv1alpha2.SectionName(hostname), + Name: gatewayv1alpha2.SectionName("kni-"), Hostname: (*gatewayv1alpha2.Hostname)(&hostname), Port: 443, Protocol: "HTTPS", @@ -535,6 +594,10 @@ func tlsListener(hostname, nsName, secretName string) GatewayOption { } } +var withFinalizer = func(i *v1alpha1.Ingress) { + i.Finalizers = append(i.Finalizers, "ingresses.networking.internal.knative.dev") +} + func withTLS(secret string) IngressOption { return func(i *v1alpha1.Ingress) { i.Spec.TLS = append(i.Spec.TLS, v1alpha1.IngressTLS{ diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index 7d4d7a438..8b9e9c7dc 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -34,6 +34,8 @@ import ( "knative.dev/pkg/controller" ) +const listenerPrefix = "kni-" + // reconcileHTTPRoute reconciles HTTPRoute. func (c *Reconciler) reconcileHTTPRoute( ctx context.Context, ing *netv1alpha1.Ingress, @@ -154,7 +156,7 @@ func (c *Reconciler) reconcileTLS( for _, h := range tls.Hosts { h := h listener := gatewayv1alpha2.Listener{ - Name: gatewayv1alpha2.SectionName(h), + Name: gatewayv1alpha2.SectionName(listenerPrefix + ing.GetUID()), Hostname: (*gatewayv1alpha2.Hostname)(&h), Port: 443, Protocol: gatewayv1alpha2.HTTPSProtocolType, @@ -241,3 +243,39 @@ func (c *Reconciler) reconcileGatewayListeners( return nil } + +func (c *Reconciler) clearGatewayListeners(ctx context.Context, ing *netv1alpha1.Ingress, gwName *types.NamespacedName) error { + recorder := controller.GetEventRecorder(ctx) + + gw, err := c.gatewayLister.Gateways(gwName.Namespace).Get(gwName.Name) + if apierrs.IsNotFound(err) { + // Nothing to clean up, all done! + return nil + } else if err != nil { + return err + } + + listenerName := listenerPrefix + string(ing.GetUID()) + update := gw.DeepCopy() + + numListeners := len(update.Spec.Listeners) + for i := numListeners - 1; i >= 0; i-- { + // March backwards down the list removing items by swapping in the last item and trimming the list + // A generic list.remove(func) would be nice here. + l := update.Spec.Listeners[i] + if string(l.Name) == string(listenerName) { + update.Spec.Listeners[i] = update.Spec.Listeners[len(update.Spec.Listeners)-1] + update.Spec.Listeners = update.Spec.Listeners[:len(update.Spec.Listeners)-1] + } + } + + if len(update.Spec.Listeners) != numListeners { + _, err := c.gwapiclient.GatewayV1alpha2().Gateways(update.Namespace).Update(ctx, update, metav1.UpdateOptions{}) + if err != nil { + recorder.Eventf(ing, corev1.EventTypeWarning, "GatewayUpdateFailed", "Failed to remove Listener from Gateway %s: %v", gwName, err) + return fmt.Errorf("failed to update Gateway %s/%s: %w", update.Namespace, update.Name, err) + } + } + + return nil +} diff --git a/pkg/reconciler/ingress/resources/httproute.go b/pkg/reconciler/ingress/resources/httproute.go index 788909a70..17a91eb89 100644 --- a/pkg/reconciler/ingress/resources/httproute.go +++ b/pkg/reconciler/ingress/resources/httproute.go @@ -75,6 +75,8 @@ func makeHTTPRouteSpec( namespacedNameGateway := gatewayConfig.Gateways[rule.Visibility].Gateway gatewayRef := gatewayv1alpha2.ParentRef{ + Group: (*gatewayv1alpha2.Group)(&gatewayv1alpha2.GroupVersion.Group), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Gateway")), Namespace: namespacePtr(gatewayv1alpha2.Namespace(namespacedNameGateway.Namespace)), Name: gatewayv1alpha2.ObjectName(namespacedNameGateway.Name), } @@ -132,8 +134,10 @@ func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayv1alpha2.HTTPRout backendRef := gatewayv1alpha2.HTTPBackendRef{ BackendRef: gatewayv1alpha2.BackendRef{ BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ - Port: portNumPtr(split.ServicePort.IntValue()), - Name: gatewayv1alpha2.ObjectName(name), + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Service")), + Port: portNumPtr(split.ServicePort.IntValue()), + Name: gatewayv1alpha2.ObjectName(name), }, Weight: pointer.Int32Ptr(int32(split.Percent)), }, @@ -155,7 +159,7 @@ func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayv1alpha2.HTTPRout Value: pointer.StringPtr(pathPrefix), } - headerMatchList := []gatewayv1alpha2.HTTPHeaderMatch{} + var headerMatchList []gatewayv1alpha2.HTTPHeaderMatch for k, v := range path.Headers { headerMatch := gatewayv1alpha2.HTTPHeaderMatch{ Type: headerMatchTypePtr(gatewayv1alpha2.HeaderMatchExact), diff --git a/pkg/reconciler/ingress/resources/httproute_test.go b/pkg/reconciler/ingress/resources/httproute_test.go index a55995d84..9e2bb6f61 100644 --- a/pkg/reconciler/ingress/resources/httproute_test.go +++ b/pkg/reconciler/ingress/resources/httproute_test.go @@ -152,8 +152,10 @@ func TestMakeHTTPRoute(t *testing.T) { BackendRefs: []gatewayv1alpha2.HTTPBackendRef{{ BackendRef: gatewayv1alpha2.BackendRef{ BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ - Port: portNumPtr(123), - Name: gatewayv1alpha2.ObjectName("goo"), + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Service")), + Name: gatewayv1alpha2.ObjectName("goo"), + Port: portNumPtr(123), }, Weight: pointer.Int32Ptr(int32(12)), }, @@ -173,8 +175,10 @@ func TestMakeHTTPRoute(t *testing.T) { }, { BackendRef: gatewayv1alpha2.BackendRef{ BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ - Port: portNumPtr(124), - Name: gatewayv1alpha2.ObjectName("doo"), + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Service")), + Port: portNumPtr(124), + Name: gatewayv1alpha2.ObjectName("doo"), }, Weight: pointer.Int32Ptr(int32(88)), }, @@ -205,12 +209,13 @@ func TestMakeHTTPRoute(t *testing.T) { Type: pathMatchTypePtr(gatewayv1alpha2.PathMatchPathPrefix), Value: pointer.StringPtr("/"), }, - Headers: []gatewayv1alpha2.HTTPHeaderMatch{}, }, }, }}, CommonRouteSpec: gatewayv1alpha2.CommonRouteSpec{ ParentRefs: []gatewayv1alpha2.ParentRef{{ + Group: (*gatewayv1alpha2.Group)(pointer.String("gateway.networking.k8s.io")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Gateway")), Namespace: namespacePtr("test-ns"), Name: gatewayv1alpha2.ObjectName("foo"), }}, @@ -232,8 +237,10 @@ func TestMakeHTTPRoute(t *testing.T) { BackendRefs: []gatewayv1alpha2.HTTPBackendRef{{ BackendRef: gatewayv1alpha2.BackendRef{ BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ - Port: portNumPtr(123), - Name: gatewayv1alpha2.ObjectName("goo"), + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Service")), + Port: portNumPtr(123), + Name: gatewayv1alpha2.ObjectName("goo"), }, Weight: pointer.Int32Ptr(int32(12)), }, @@ -253,8 +260,10 @@ func TestMakeHTTPRoute(t *testing.T) { }, { BackendRef: gatewayv1alpha2.BackendRef{ BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ - Port: portNumPtr(124), - Name: gatewayv1alpha2.ObjectName("doo"), + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Service")), + Port: portNumPtr(124), + Name: gatewayv1alpha2.ObjectName("doo"), }, Weight: pointer.Int32Ptr(int32(88)), }, @@ -284,11 +293,12 @@ func TestMakeHTTPRoute(t *testing.T) { Type: pathMatchTypePtr(gatewayv1alpha2.PathMatchPathPrefix), Value: pointer.StringPtr("/"), }, - Headers: []gatewayv1alpha2.HTTPHeaderMatch{}, }}, }}, CommonRouteSpec: gatewayv1alpha2.CommonRouteSpec{ ParentRefs: []gatewayv1alpha2.ParentRef{{ + Group: (*gatewayv1alpha2.Group)(pointer.String("gateway.networking.k8s.io")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Gateway")), Namespace: namespacePtr("test-ns"), Name: gatewayv1alpha2.ObjectName("foo-local"), }}, @@ -358,8 +368,10 @@ func TestMakeHTTPRoute(t *testing.T) { BackendRefs: []gatewayv1alpha2.HTTPBackendRef{{ BackendRef: gatewayv1alpha2.BackendRef{ BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ - Port: portNumPtr(123), - Name: gatewayv1alpha2.ObjectName("goo"), + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Service")), + Port: portNumPtr(123), + Name: gatewayv1alpha2.ObjectName("goo"), }, Weight: pointer.Int32Ptr(int32(100)), }, @@ -384,8 +396,10 @@ func TestMakeHTTPRoute(t *testing.T) { BackendRefs: []gatewayv1alpha2.HTTPBackendRef{{ BackendRef: gatewayv1alpha2.BackendRef{ BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ - Port: portNumPtr(124), - Name: gatewayv1alpha2.ObjectName("doo"), + Group: (*gatewayv1alpha2.Group)(pointer.String("")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Service")), + Port: portNumPtr(124), + Name: gatewayv1alpha2.ObjectName("doo"), }, Weight: pointer.Int32Ptr(int32(100)), }, @@ -410,6 +424,8 @@ func TestMakeHTTPRoute(t *testing.T) { }, CommonRouteSpec: gatewayv1alpha2.CommonRouteSpec{ ParentRefs: []gatewayv1alpha2.ParentRef{{ + Group: (*gatewayv1alpha2.Group)(pointer.String("gateway.networking.k8s.io")), + Kind: (*gatewayv1alpha2.Kind)(pointer.String("Gateway")), Namespace: namespacePtr("test-ns"), Name: gatewayv1alpha2.ObjectName("foo"), }}, From f24416567271179f11587a850ab696a6bde153bf Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Thu, 7 Jul 2022 19:04:27 -0400 Subject: [PATCH 07/10] Fix lint complaint. --- pkg/reconciler/ingress/reconcile_resources.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index 8b9e9c7dc..b742944f8 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -263,7 +263,7 @@ func (c *Reconciler) clearGatewayListeners(ctx context.Context, ing *netv1alpha1 // March backwards down the list removing items by swapping in the last item and trimming the list // A generic list.remove(func) would be nice here. l := update.Spec.Listeners[i] - if string(l.Name) == string(listenerName) { + if string(l.Name) == listenerName { update.Spec.Listeners[i] = update.Spec.Listeners[len(update.Spec.Listeners)-1] update.Spec.Listeners = update.Spec.Listeners[:len(update.Spec.Listeners)-1] } From 4045dceddbbf0734e46d92993d01a1b0103c95d5 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 11 Jul 2022 18:23:47 -0400 Subject: [PATCH 08/10] Fix test breakage after merge post-#315 --- pkg/reconciler/ingress/controller.go | 12 +++++------ pkg/reconciler/ingress/controller_test.go | 2 +- pkg/reconciler/ingress/ingress.go | 2 +- pkg/reconciler/ingress/ingress_test.go | 20 +++++++++---------- pkg/reconciler/ingress/reconcile_resources.go | 20 +++++++++---------- .../ingress/resources/reference_grant.go | 11 +++++----- pkg/reconciler/testing/listers.go | 4 ++-- 7 files changed, 35 insertions(+), 36 deletions(-) diff --git a/pkg/reconciler/ingress/controller.go b/pkg/reconciler/ingress/controller.go index c5db4b65b..4c0266d94 100644 --- a/pkg/reconciler/ingress/controller.go +++ b/pkg/reconciler/ingress/controller.go @@ -37,7 +37,7 @@ import ( gwapiclient "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/client" gatewayinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/gateway" httprouteinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/httproute" - referencepolicyinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencepolicy" + referencegrantinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencegrant" "knative.dev/net-gateway-api/pkg/reconciler/ingress/config" ) @@ -56,15 +56,15 @@ func NewController( ingressInformer := ingressinformer.Get(ctx) httprouteInformer := httprouteinformer.Get(ctx) - referencePolicyInformer := referencepolicyinformer.Get(ctx) + referenceGrantInformer := referencegrantinformer.Get(ctx) gatewayInformer := gatewayinformer.Get(ctx) endpointsInformer := endpointsinformer.Get(ctx) c := &Reconciler{ - gwapiclient: gwapiclient.Get(ctx), - httprouteLister: httprouteInformer.Lister(), - referencePolicyLister: referencePolicyInformer.Lister(), - gatewayLister: gatewayInformer.Lister(), + gwapiclient: gwapiclient.Get(ctx), + httprouteLister: httprouteInformer.Lister(), + referenceGrantLister: referenceGrantInformer.Lister(), + gatewayLister: gatewayInformer.Lister(), } filterFunc := reconciler.AnnotationFilterFunc(networking.IngressClassAnnotationKey, gatewayAPIIngressClassName, false) diff --git a/pkg/reconciler/ingress/controller_test.go b/pkg/reconciler/ingress/controller_test.go index eebaad901..2515a09ba 100644 --- a/pkg/reconciler/ingress/controller_test.go +++ b/pkg/reconciler/ingress/controller_test.go @@ -32,7 +32,7 @@ import ( _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/gateway/fake" _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/httproute/fake" - _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencepolicy/fake" + _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencegrant/fake" "knative.dev/net-gateway-api/pkg/reconciler/ingress/config" ) diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index 15d8961b2..1066a4e52 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -49,7 +49,7 @@ type Reconciler struct { // Listers index properties about resources httprouteLister gatewayListers.HTTPRouteLister - referencePolicyLister gatewayListers.ReferencePolicyLister + referenceGrantLister gatewayListers.ReferenceGrantLister gatewayLister gatewayListers.GatewayLister } diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index fb86a1b18..42a77cc6f 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -392,10 +392,10 @@ func TestReconcileTLS(t *testing.T) { table.Test(t, GatewayFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher, tr *TableRow) controller.Reconciler { r := &Reconciler{ - gwapiclient: fakegwapiclientset.Get(ctx), - httprouteLister: listers.GetHTTPRouteLister(), - referencePolicyLister: listers.GetReferencePolicyLister(), - gatewayLister: listers.GetGatewayLister(), + gwapiclient: fakegwapiclientset.Get(ctx), + httprouteLister: listers.GetHTTPRouteLister(), + referenceGrantLister: listers.GetReferenceGrantLister(), + gatewayLister: listers.GetGatewayLister(), statusManager: &fakeStatusManager{FakeIsReady: func(context.Context, *v1alpha1.Ingress) (bool, error) { return true, nil }}, @@ -571,7 +571,7 @@ func tlsListener(hostname, nsName, secretName string) GatewayOption { Protocol: "HTTPS", TLS: &gatewayv1alpha2.GatewayTLSConfig{ Mode: (*gatewayv1alpha2.TLSModeType)(pointer.String("Terminate")), - CertificateRefs: []*gatewayv1alpha2.SecretObjectReference{{ + CertificateRefs: []gatewayv1alpha2.SecretObjectReference{{ Group: (*gatewayv1alpha2.Group)(pointer.String("")), Kind: (*gatewayv1alpha2.Kind)(pointer.String("Secret")), Name: gatewayv1alpha2.ObjectName(secretName), @@ -622,9 +622,9 @@ func secret(name, ns string) *corev1.Secret { } } -func rp(to *corev1.Secret) *gatewayv1alpha2.ReferencePolicy { +func rp(to *corev1.Secret) *gatewayv1alpha2.ReferenceGrant { t := true - return &gatewayv1alpha2.ReferencePolicy{ + return &gatewayv1alpha2.ReferenceGrant{ ObjectMeta: metav1.ObjectMeta{ Name: to.Name + "-" + testNamespace, Namespace: to.Namespace, @@ -636,13 +636,13 @@ func rp(to *corev1.Secret) *gatewayv1alpha2.ReferencePolicy { BlockOwnerDeletion: &t, }}, }, - Spec: gatewayv1alpha2.ReferencePolicySpec{ - From: []gatewayv1alpha2.ReferencePolicyFrom{{ + Spec: gatewayv1alpha2.ReferenceGrantSpec{ + From: []gatewayv1alpha2.ReferenceGrantFrom{{ Group: "gateway.networking.k8s.io", Kind: "Gateway", Namespace: gatewayv1alpha2.Namespace(testNamespace), }}, - To: []gatewayv1alpha2.ReferencePolicyTo{{ + To: []gatewayv1alpha2.ReferenceGrantTo{{ Group: gatewayv1alpha2.Group(""), Kind: gatewayv1alpha2.Kind("Secret"), Name: (*gatewayv1alpha2.ObjectName)(&to.Name), diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index b742944f8..f9ba3e456 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -119,32 +119,32 @@ func (c *Reconciler) reconcileTLS( desired := resources.MakeReferenceGrant(ctx, ing, secret, gateway) - rp, err := c.referencePolicyLister.ReferencePolicies(desired.Namespace).Get(desired.Name) + rp, err := c.referenceGrantLister.ReferenceGrants(desired.Namespace).Get(desired.Name) if apierrs.IsNotFound(err) { - rp, err = c.gwapiclient.GatewayV1alpha2().ReferencePolicies(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{}) + rp, err = c.gwapiclient.GatewayV1alpha2().ReferenceGrants(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{}) if err != nil { - recorder.Eventf(ing, corev1.EventTypeWarning, "CreationFailed", "Failed to create ReferencePolicy: %v", err) - return nil, fmt.Errorf("failed to create ReferencePolicy: %w", err) + recorder.Eventf(ing, corev1.EventTypeWarning, "CreationFailed", "Failed to create ReferenceGrant: %v", err) + return nil, fmt.Errorf("failed to create ReferenceGrant: %w", err) } } else if err != nil { return nil, err } if !metav1.IsControlledBy(rp, ing) { - recorder.Eventf(ing, corev1.EventTypeWarning, "NotOwned", "ReferencePolicy %s not owned by this object", desired.Name) - return nil, fmt.Errorf("ReferencePolicy %s not owned by %s", rp.Name, ing.Name) + recorder.Eventf(ing, corev1.EventTypeWarning, "NotOwned", "ReferenceGrant %s not owned by this object", desired.Name) + return nil, fmt.Errorf("ReferenceGrant %s not owned by %s", rp.Name, ing.Name) } if !equality.Semantic.DeepEqual(rp.Spec, desired.Spec) { update := rp.DeepCopy() update.Spec = desired.Spec - _, err := c.gwapiclient.GatewayV1alpha2().ReferencePolicies(update.Namespace).Update(ctx, update, metav1.UpdateOptions{}) + _, err := c.gwapiclient.GatewayV1alpha2().ReferenceGrants(update.Namespace).Update(ctx, update, metav1.UpdateOptions{}) if err != nil { - recorder.Eventf(ing, corev1.EventTypeWarning, "UpdateFailed", "Failed to update ReferencePolicy: %v", err) - return nil, fmt.Errorf("failed to update ReferencePolicy: %w", err) + recorder.Eventf(ing, corev1.EventTypeWarning, "UpdateFailed", "Failed to update ReferenceGrant: %v", err) + return nil, fmt.Errorf("failed to update ReferenceGrant: %w", err) } } @@ -162,7 +162,7 @@ func (c *Reconciler) reconcileTLS( Protocol: gatewayv1alpha2.HTTPSProtocolType, TLS: &gatewayv1alpha2.GatewayTLSConfig{ Mode: &mode, - CertificateRefs: []*gatewayv1alpha2.SecretObjectReference{{ + CertificateRefs: []gatewayv1alpha2.SecretObjectReference{{ Group: (*gatewayv1alpha2.Group)(pointer.String("")), Kind: (*gatewayv1alpha2.Kind)(pointer.String("Secret")), Name: gatewayv1alpha2.ObjectName(tls.SecretName), diff --git a/pkg/reconciler/ingress/resources/reference_grant.go b/pkg/reconciler/ingress/resources/reference_grant.go index 42172fff6..b2e605b14 100644 --- a/pkg/reconciler/ingress/resources/reference_grant.go +++ b/pkg/reconciler/ingress/resources/reference_grant.go @@ -26,15 +26,14 @@ import ( ) // Grant the resource "to" access to the resource "from" -// N.B. ReferencePolicy will be renamed ReferenceGrant in v1beta1 -func MakeReferenceGrant(ctx context.Context, ing *netv1alpha1.Ingress, to, from metav1.PartialObjectMetadata) *gatewayv1alpha2.ReferencePolicy { +func MakeReferenceGrant(ctx context.Context, ing *netv1alpha1.Ingress, to, from metav1.PartialObjectMetadata) *gatewayv1alpha2.ReferenceGrant { name := to.Name if len(name)+len(from.Namespace) > 62 { name = name[:62-len(from.Namespace)] } name += "-" + from.Namespace - retval := &gatewayv1alpha2.ReferencePolicy{ + retval := &gatewayv1alpha2.ReferenceGrant{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: to.Namespace, @@ -42,13 +41,13 @@ func MakeReferenceGrant(ctx context.Context, ing *netv1alpha1.Ingress, to, from Annotations: to.Annotations, OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)}, }, - Spec: gatewayv1alpha2.ReferencePolicySpec{ - From: []gatewayv1alpha2.ReferencePolicyFrom{{ + Spec: gatewayv1alpha2.ReferenceGrantSpec{ + From: []gatewayv1alpha2.ReferenceGrantFrom{{ Group: gatewayv1alpha2.Group(from.GroupVersionKind().Group), Kind: gatewayv1alpha2.Kind(from.Kind), Namespace: gatewayv1alpha2.Namespace(from.Namespace), }}, - To: []gatewayv1alpha2.ReferencePolicyTo{{ + To: []gatewayv1alpha2.ReferenceGrantTo{{ Group: gatewayv1alpha2.Group(to.GroupVersionKind().Group), Kind: gatewayv1alpha2.Kind(to.Kind), Name: (*gatewayv1alpha2.ObjectName)(&to.Name), diff --git a/pkg/reconciler/testing/listers.go b/pkg/reconciler/testing/listers.go index bfe963f12..b7c4a5a29 100644 --- a/pkg/reconciler/testing/listers.go +++ b/pkg/reconciler/testing/listers.go @@ -104,6 +104,6 @@ func (l *Listers) GetGatewayLister() gatewayListers.GatewayLister { return gatewayListers.NewGatewayLister(l.IndexerFor(&gatewayv1alpa2.Gateway{})) } -func (l *Listers) GetReferencePolicyLister() gatewayListers.ReferencePolicyLister { - return gatewayListers.NewReferencePolicyLister(l.IndexerFor(&gatewayv1alpa2.ReferencePolicy{})) +func (l *Listers) GetReferenceGrantLister() gatewayListers.ReferenceGrantLister { + return gatewayListers.NewReferenceGrantLister(l.IndexerFor(&gatewayv1alpa2.ReferenceGrant{})) } From 77411959c9e6e9f8e114264baec15c40c199188c Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 11 Jul 2022 19:16:41 -0400 Subject: [PATCH 09/10] Update referencepolicy->referencegrant in roles as well --- config/200-clusterrole.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/200-clusterrole.yaml b/config/200-clusterrole.yaml index f99eb9763..b55d3d502 100644 --- a/config/200-clusterrole.yaml +++ b/config/200-clusterrole.yaml @@ -39,7 +39,7 @@ metadata: app.kubernetes.io/version: devel rules: - apiGroups: ["gateway.networking.k8s.io"] - resources: ["httproutes", "referencepolicies"] + resources: ["httproutes", "referencegrants"] verbs: ["get", "list", "create", "update", "delete", "patch", "watch"] - apiGroups: ["gateway.networking.k8s.io"] resources: ["gateways"] From ad027c7324cadf13812233eb63500550333a7a47 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 12 Jul 2022 09:37:36 -0400 Subject: [PATCH 10/10] Switch back to ReferencePolicy from ReferenceGrant, because Istio doesn't support ReferenceGrant yet, and there's no converter. --- config/200-clusterrole.yaml | 2 +- pkg/reconciler/ingress/controller.go | 12 +++---- pkg/reconciler/ingress/controller_test.go | 2 +- pkg/reconciler/ingress/ingress.go | 2 +- pkg/reconciler/ingress/ingress_test.go | 12 +++---- pkg/reconciler/ingress/reconcile_resources.go | 18 +++++----- .../ingress/resources/reference_grant.go | 33 ++++++++++--------- pkg/reconciler/testing/listers.go | 4 +-- 8 files changed, 44 insertions(+), 41 deletions(-) diff --git a/config/200-clusterrole.yaml b/config/200-clusterrole.yaml index b55d3d502..be31a40cf 100644 --- a/config/200-clusterrole.yaml +++ b/config/200-clusterrole.yaml @@ -39,7 +39,7 @@ metadata: app.kubernetes.io/version: devel rules: - apiGroups: ["gateway.networking.k8s.io"] - resources: ["httproutes", "referencegrants"] + resources: ["httproutes", "referencegrants", "referencepolicies"] verbs: ["get", "list", "create", "update", "delete", "patch", "watch"] - apiGroups: ["gateway.networking.k8s.io"] resources: ["gateways"] diff --git a/pkg/reconciler/ingress/controller.go b/pkg/reconciler/ingress/controller.go index 4c0266d94..c5db4b65b 100644 --- a/pkg/reconciler/ingress/controller.go +++ b/pkg/reconciler/ingress/controller.go @@ -37,7 +37,7 @@ import ( gwapiclient "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/client" gatewayinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/gateway" httprouteinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/httproute" - referencegrantinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencegrant" + referencepolicyinformer "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencepolicy" "knative.dev/net-gateway-api/pkg/reconciler/ingress/config" ) @@ -56,15 +56,15 @@ func NewController( ingressInformer := ingressinformer.Get(ctx) httprouteInformer := httprouteinformer.Get(ctx) - referenceGrantInformer := referencegrantinformer.Get(ctx) + referencePolicyInformer := referencepolicyinformer.Get(ctx) gatewayInformer := gatewayinformer.Get(ctx) endpointsInformer := endpointsinformer.Get(ctx) c := &Reconciler{ - gwapiclient: gwapiclient.Get(ctx), - httprouteLister: httprouteInformer.Lister(), - referenceGrantLister: referenceGrantInformer.Lister(), - gatewayLister: gatewayInformer.Lister(), + gwapiclient: gwapiclient.Get(ctx), + httprouteLister: httprouteInformer.Lister(), + referencePolicyLister: referencePolicyInformer.Lister(), + gatewayLister: gatewayInformer.Lister(), } filterFunc := reconciler.AnnotationFilterFunc(networking.IngressClassAnnotationKey, gatewayAPIIngressClassName, false) diff --git a/pkg/reconciler/ingress/controller_test.go b/pkg/reconciler/ingress/controller_test.go index 2515a09ba..eebaad901 100644 --- a/pkg/reconciler/ingress/controller_test.go +++ b/pkg/reconciler/ingress/controller_test.go @@ -32,7 +32,7 @@ import ( _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/gateway/fake" _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/httproute/fake" - _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencegrant/fake" + _ "knative.dev/net-gateway-api/pkg/client/gatewayapi/injection/informers/apis/v1alpha2/referencepolicy/fake" "knative.dev/net-gateway-api/pkg/reconciler/ingress/config" ) diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index 1066a4e52..15d8961b2 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -49,7 +49,7 @@ type Reconciler struct { // Listers index properties about resources httprouteLister gatewayListers.HTTPRouteLister - referenceGrantLister gatewayListers.ReferenceGrantLister + referencePolicyLister gatewayListers.ReferencePolicyLister gatewayLister gatewayListers.GatewayLister } diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index 42a77cc6f..393386a45 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -392,10 +392,10 @@ func TestReconcileTLS(t *testing.T) { table.Test(t, GatewayFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher, tr *TableRow) controller.Reconciler { r := &Reconciler{ - gwapiclient: fakegwapiclientset.Get(ctx), - httprouteLister: listers.GetHTTPRouteLister(), - referenceGrantLister: listers.GetReferenceGrantLister(), - gatewayLister: listers.GetGatewayLister(), + gwapiclient: fakegwapiclientset.Get(ctx), + httprouteLister: listers.GetHTTPRouteLister(), + referencePolicyLister: listers.GetReferencePolicyLister(), + gatewayLister: listers.GetGatewayLister(), statusManager: &fakeStatusManager{FakeIsReady: func(context.Context, *v1alpha1.Ingress) (bool, error) { return true, nil }}, @@ -622,9 +622,9 @@ func secret(name, ns string) *corev1.Secret { } } -func rp(to *corev1.Secret) *gatewayv1alpha2.ReferenceGrant { +func rp(to *corev1.Secret) *gatewayv1alpha2.ReferencePolicy { t := true - return &gatewayv1alpha2.ReferenceGrant{ + return &gatewayv1alpha2.ReferencePolicy{ ObjectMeta: metav1.ObjectMeta{ Name: to.Name + "-" + testNamespace, Namespace: to.Namespace, diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index f9ba3e456..46409ec63 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -119,32 +119,32 @@ func (c *Reconciler) reconcileTLS( desired := resources.MakeReferenceGrant(ctx, ing, secret, gateway) - rp, err := c.referenceGrantLister.ReferenceGrants(desired.Namespace).Get(desired.Name) + rp, err := c.referencePolicyLister.ReferencePolicies(desired.Namespace).Get(desired.Name) if apierrs.IsNotFound(err) { - rp, err = c.gwapiclient.GatewayV1alpha2().ReferenceGrants(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{}) + rp, err = c.gwapiclient.GatewayV1alpha2().ReferencePolicies(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{}) if err != nil { - recorder.Eventf(ing, corev1.EventTypeWarning, "CreationFailed", "Failed to create ReferenceGrant: %v", err) - return nil, fmt.Errorf("failed to create ReferenceGrant: %w", err) + recorder.Eventf(ing, corev1.EventTypeWarning, "CreationFailed", "Failed to create ReferencePolicy: %v", err) + return nil, fmt.Errorf("failed to create ReferencePolicy: %w", err) } } else if err != nil { return nil, err } if !metav1.IsControlledBy(rp, ing) { - recorder.Eventf(ing, corev1.EventTypeWarning, "NotOwned", "ReferenceGrant %s not owned by this object", desired.Name) - return nil, fmt.Errorf("ReferenceGrant %s not owned by %s", rp.Name, ing.Name) + recorder.Eventf(ing, corev1.EventTypeWarning, "NotOwned", "ReferencePolicy %s not owned by this object", desired.Name) + return nil, fmt.Errorf("ReferencePolicy %s not owned by %s", rp.Name, ing.Name) } if !equality.Semantic.DeepEqual(rp.Spec, desired.Spec) { update := rp.DeepCopy() update.Spec = desired.Spec - _, err := c.gwapiclient.GatewayV1alpha2().ReferenceGrants(update.Namespace).Update(ctx, update, metav1.UpdateOptions{}) + _, err := c.gwapiclient.GatewayV1alpha2().ReferencePolicies(update.Namespace).Update(ctx, update, metav1.UpdateOptions{}) if err != nil { - recorder.Eventf(ing, corev1.EventTypeWarning, "UpdateFailed", "Failed to update ReferenceGrant: %v", err) - return nil, fmt.Errorf("failed to update ReferenceGrant: %w", err) + recorder.Eventf(ing, corev1.EventTypeWarning, "UpdateFailed", "Failed to update ReferencePolicy: %v", err) + return nil, fmt.Errorf("failed to update ReferencePolicy: %w", err) } } diff --git a/pkg/reconciler/ingress/resources/reference_grant.go b/pkg/reconciler/ingress/resources/reference_grant.go index b2e605b14..c23ab7ffd 100644 --- a/pkg/reconciler/ingress/resources/reference_grant.go +++ b/pkg/reconciler/ingress/resources/reference_grant.go @@ -26,14 +26,28 @@ import ( ) // Grant the resource "to" access to the resource "from" -func MakeReferenceGrant(ctx context.Context, ing *netv1alpha1.Ingress, to, from metav1.PartialObjectMetadata) *gatewayv1alpha2.ReferenceGrant { +// TODO: remove ReferencePolicy return value once Istio supports ReferenceGrant +func MakeReferenceGrant(ctx context.Context, ing *netv1alpha1.Ingress, to, from metav1.PartialObjectMetadata) *gatewayv1alpha2.ReferencePolicy { name := to.Name if len(name)+len(from.Namespace) > 62 { name = name[:62-len(from.Namespace)] } name += "-" + from.Namespace - retval := &gatewayv1alpha2.ReferenceGrant{ + spec := gatewayv1alpha2.ReferenceGrantSpec{ + From: []gatewayv1alpha2.ReferenceGrantFrom{{ + Group: gatewayv1alpha2.Group(from.GroupVersionKind().Group), + Kind: gatewayv1alpha2.Kind(from.Kind), + Namespace: gatewayv1alpha2.Namespace(from.Namespace), + }}, + To: []gatewayv1alpha2.ReferenceGrantTo{{ + Group: gatewayv1alpha2.Group(to.GroupVersionKind().Group), + Kind: gatewayv1alpha2.Kind(to.Kind), + Name: (*gatewayv1alpha2.ObjectName)(&to.Name), + }}, + } + + legacy := &gatewayv1alpha2.ReferencePolicy{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: to.Namespace, @@ -41,19 +55,8 @@ func MakeReferenceGrant(ctx context.Context, ing *netv1alpha1.Ingress, to, from Annotations: to.Annotations, OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)}, }, - Spec: gatewayv1alpha2.ReferenceGrantSpec{ - From: []gatewayv1alpha2.ReferenceGrantFrom{{ - Group: gatewayv1alpha2.Group(from.GroupVersionKind().Group), - Kind: gatewayv1alpha2.Kind(from.Kind), - Namespace: gatewayv1alpha2.Namespace(from.Namespace), - }}, - To: []gatewayv1alpha2.ReferenceGrantTo{{ - Group: gatewayv1alpha2.Group(to.GroupVersionKind().Group), - Kind: gatewayv1alpha2.Kind(to.Kind), - Name: (*gatewayv1alpha2.ObjectName)(&to.Name), - }}, - }, + Spec: spec, } - return retval + return legacy } diff --git a/pkg/reconciler/testing/listers.go b/pkg/reconciler/testing/listers.go index b7c4a5a29..bfe963f12 100644 --- a/pkg/reconciler/testing/listers.go +++ b/pkg/reconciler/testing/listers.go @@ -104,6 +104,6 @@ func (l *Listers) GetGatewayLister() gatewayListers.GatewayLister { return gatewayListers.NewGatewayLister(l.IndexerFor(&gatewayv1alpa2.Gateway{})) } -func (l *Listers) GetReferenceGrantLister() gatewayListers.ReferenceGrantLister { - return gatewayListers.NewReferenceGrantLister(l.IndexerFor(&gatewayv1alpa2.ReferenceGrant{})) +func (l *Listers) GetReferencePolicyLister() gatewayListers.ReferencePolicyLister { + return gatewayListers.NewReferencePolicyLister(l.IndexerFor(&gatewayv1alpa2.ReferencePolicy{})) }